From c1ef7ed843ce0daead8eaa3af8a75d5bc8ba3ee8 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Fri, 10 Jun 2022 22:34:34 +0000 Subject: [PATCH 1/4] refactor: fix type annotations in open There was no clear reason as to why we needed to use type assertions when initializing both `args` and `options` in `open` so I refactored them both. --- src/node/util.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/node/util.ts b/src/node/util.ts index 37eefc70787b..f5bf0ea71cf7 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -416,8 +416,8 @@ export const open = async (address: URL | string): Promise => { if (url.hostname === "0.0.0.0") { url.hostname = "localhost" } - const args = [] as string[] - const options = {} as cp.SpawnOptions + const args: string[] = [] + const options: cp.SpawnOptions = {} const platform = (await isWsl(process.platform, os.release(), "/proc/version")) ? "wsl" : process.platform let command = platform === "darwin" ? "open" : "xdg-open" if (platform === "win32" || platform === "wsl") { From 0abb3c3951417a6750766466da8b978504a29fda Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Fri, 10 Jun 2022 22:46:33 +0000 Subject: [PATCH 2/4] refactor: create constructOpenOptions --- src/node/util.ts | 43 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/node/util.ts b/src/node/util.ts index f5bf0ea71cf7..0c2f92f3b149 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -404,6 +404,40 @@ export const isWsl = async ( } } +interface OpenOptions { + args: string[] + options: cp.SpawnOptions + command: string + urlSearch: string +} + +/** + * A helper function to construct options for `open` function. + * + * Extract to make it easier to test. + * + * @param platform - platform on machine + * @param urlSearch - url.search + * @returns an object with args, command, options and urlSearch + */ +export function constructOpenOptions(platform: NodeJS.Platform | "wsl", urlSearch: string): OpenOptions { + const args: string[] = [] + const options: cp.SpawnOptions = {} + let command = platform === "darwin" ? "open" : "xdg-open" + if (platform === "win32" || platform === "wsl") { + command = platform === "wsl" ? "cmd.exe" : "cmd" + args.push("/c", "start", '""', "/b") + urlSearch = urlSearch.replace(/&/g, "^&") + } + + return { + args, + command, + options, + urlSearch, + } +} + /** * Try opening an address using whatever the system has set for opening URLs. */ @@ -416,15 +450,8 @@ export const open = async (address: URL | string): Promise => { if (url.hostname === "0.0.0.0") { url.hostname = "localhost" } - const args: string[] = [] - const options: cp.SpawnOptions = {} const platform = (await isWsl(process.platform, os.release(), "/proc/version")) ? "wsl" : process.platform - let command = platform === "darwin" ? "open" : "xdg-open" - if (platform === "win32" || platform === "wsl") { - command = platform === "wsl" ? "cmd.exe" : "cmd" - args.push("/c", "start", '""', "/b") - url.search = url.search.replace(/&/g, "^&") - } + const { command, args, options } = constructOpenOptions(platform, url.search) const proc = cp.spawn(command, [...args, url.toString()], options) await new Promise((resolve, reject) => { proc.on("error", reject) From 7f0c24d9264f441d894fe5f83d8bb45993380e4e Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Fri, 10 Jun 2022 22:47:54 +0000 Subject: [PATCH 3/4] refactor: add urlSearch and remove options --- src/node/util.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/node/util.ts b/src/node/util.ts index 0c2f92f3b149..140431cc860c 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -406,7 +406,6 @@ export const isWsl = async ( interface OpenOptions { args: string[] - options: cp.SpawnOptions command: string urlSearch: string } @@ -422,7 +421,6 @@ interface OpenOptions { */ export function constructOpenOptions(platform: NodeJS.Platform | "wsl", urlSearch: string): OpenOptions { const args: string[] = [] - const options: cp.SpawnOptions = {} let command = platform === "darwin" ? "open" : "xdg-open" if (platform === "win32" || platform === "wsl") { command = platform === "wsl" ? "cmd.exe" : "cmd" @@ -433,7 +431,6 @@ export function constructOpenOptions(platform: NodeJS.Platform | "wsl", urlSearc return { args, command, - options, urlSearch, } } @@ -451,8 +448,9 @@ export const open = async (address: URL | string): Promise => { url.hostname = "localhost" } const platform = (await isWsl(process.platform, os.release(), "/proc/version")) ? "wsl" : process.platform - const { command, args, options } = constructOpenOptions(platform, url.search) - const proc = cp.spawn(command, [...args, url.toString()], options) + const { command, args, urlSearch } = constructOpenOptions(platform, url.search) + url.search = urlSearch + const proc = cp.spawn(command, [...args, url.toString()], {}) await new Promise((resolve, reject) => { proc.on("error", reject) proc.on("close", (code) => { From ab49daf389ccc870a542ec053ada24d73eeac214 Mon Sep 17 00:00:00 2001 From: Joe Previte Date: Fri, 10 Jun 2022 23:03:08 +0000 Subject: [PATCH 4/4] feat: add tests for constructOpenOptions --- test/unit/node/util.test.ts | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/unit/node/util.test.ts b/test/unit/node/util.test.ts index 0237296e52af..f0bea5cecaf2 100644 --- a/test/unit/node/util.test.ts +++ b/test/unit/node/util.test.ts @@ -537,3 +537,38 @@ describe("isWsl", () => { }) }) }) + +describe("constructOpenOptions", () => { + it("should return options for darwin", () => { + const platform: NodeJS.Platform | "wsl" = "darwin" + const url = new URL("localhost:8080") + const { args, command, urlSearch } = util.constructOpenOptions(platform, url.search) + expect(args).toStrictEqual([]) + expect(command).toBe("open") + expect(urlSearch).toBe("") + }) + it("should return options for linux", () => { + const platform: NodeJS.Platform | "wsl" = "linux" + const url = new URL("localhost:8080") + const { args, command, urlSearch } = util.constructOpenOptions(platform, url.search) + expect(args).toStrictEqual([]) + expect(command).toBe("xdg-open") + expect(urlSearch).toBe("") + }) + it("should return options for win32", () => { + const platform: NodeJS.Platform | "wsl" = "win32" + const url = new URL("localhost:8080?q=&test") + const { args, command, urlSearch } = util.constructOpenOptions(platform, url.search) + expect(args).toStrictEqual(["/c", "start", '""', "/b"]) + expect(command).toBe("cmd") + expect(urlSearch).toBe("?q=^&test") + }) + it("should return options for wsl", () => { + const platform: NodeJS.Platform | "wsl" = "wsl" + const url = new URL("localhost:8080?q=&test") + const { args, command, urlSearch } = util.constructOpenOptions(platform, url.search) + expect(args).toStrictEqual(["/c", "start", '""', "/b"]) + expect(command).toBe("cmd.exe") + expect(urlSearch).toBe("?q=^&test") + }) +})