-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Windows support (in particular for the path lib) #1023
Comments
Actually, I found a relatively ok solution. In my project, I have my forked windows-compatible version of path as an optional dependency. Since if a parent project/module already has the package installed, it will use that package first, this will work fine for me. Moreover, the fork is only installed if the platform is win32 (can be specified in package.json) and thus when browserify is installed and not on win32 it will install the regular *nix version. Anyone in a similar position can find my fork at: https://github.com/BBWeb/path-browserify/tree/win-version It works fine for my use case but may not be optimal for everyone, and in some sense it feels unfortunate that one has to jump through these hoops to get shit working on windows :) |
Will fixing this bug fix #1092? Or are they unrelated? |
I think fixing windows support entails: 1) fix process.platform 2) fix the path shim 3) go through the core and ensure we use proper slashes everywhere and don't assume *nix (ie. /) style paths. So in a way, yes. |
Is this a node-wide bug? Or is it specific to browserify? I assume there are thousands of people using this software under Windows so I am surprised such a visible bug exists this far into the development process... |
Browserify specific. The problem lies with the shims (as outlined above). Specifically to your issue, there might be additional (but result fixable) issues within the core too. |
@cjblomqvist Okay, good. Thanks for the clarification. |
I've made additional strides towards fixing this issue properly, and one which only requires an update to path-browserify. You can check it out at: browserify/path-browserify#6 Note that there's another PR open which claims to fix the issue, but actually doesn't (it still relies on path.platform being available in the browser, which it isn't on default). It would be greatly appreciated if someone with rights to pull in the PR can take a look at it! I'm more than happy to assist any such effort! |
Basically, my issue is this: browserify/path-browserify#1
Summary: Easily fixed in path-browserify, but it depends on process.platform being available, which it cannot be as it is done right now since the process lib is only defining an object. I.e. in order for process.platform to be properly set there needs to be some kind of preprocessing server-side of the process script so that we can get the server's process values.
Any clues? This would probably fit best in some kind of option but right now it doesn't seem easily doable?
The text was updated successfully, but these errors were encountered: