Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix URLs in WSL #166

Merged
merged 1 commit into from Jan 29, 2020
Merged

Fix URLs in WSL #166

merged 1 commit into from Jan 29, 2020

Conversation

pluma
Copy link
Contributor

@pluma pluma commented Jan 28, 2020

Fixes #165.

Verbatim arguments only work when Node is running on Windows, not WSL. Without this fix, start will attempt to open the quoted URL, which will fail.

Fixes #165. Verbatim arguments only work when Node is running on Windows, not WSL. Without this fix, start will attempt to open the quoted URL, which will fail.
@radar
Copy link

radar commented Jan 28, 2020

@Apochilles and I tested this patch out this morning. It works!

However, we had to pin isWsl to 2.1.1 in package.json:

"is-wsl": "2.1.1"

This is because the older version is-wsl was not correctly detecting WSL v2 (understandably).

So with that correct version of is-wsl and this patch, the browser window does indeed open correctly.

Thanks for the quick patch @pluma!

@pluma
Copy link
Contributor Author

pluma commented Jan 28, 2020

Looks like open already depends on is-wsl ^2.1.0 and 2.1.1 just fixed a bug where undefined was returned instead of false on Linux (i.e. it shouldn't affect WSL). Any idea why you had to pin it?

@radar
Copy link

radar commented Jan 28, 2020

Well, I think another dependency of ours had made is-wsl be that version. Putting that line in our package.json makes it use the right version. I don't know much about JS package dependencies and how they're resolved, but I'm happy with this solution.

@pluma
Copy link
Contributor Author

pluma commented Jan 29, 2020

Interesting. Normally ^2.1.0 should force the version to match "2.1.0 and all minor and bugfix releases above", so you should only end up with 2.1.0 or 2.1.1 (latest). I'd love to look into what's going on with your package resolution, but this seems liked it'd go too far off-topic for this PR.

@sindresorhus sindresorhus merged commit d8a508b into sindresorhus:master Jan 29, 2020
@pluma pluma deleted the patch-1 branch January 29, 2020 15:31
@pluma
Copy link
Contributor Author

pluma commented Jan 29, 2020

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WSL: open fails due to quoting issue
3 participants