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
Support win32 paths #327
Comments
I looked deeper. Seems like you handle win32 correctly but only when Is there a way to temporarily enable win32 mode? Would it be possible to inspect paths to see if there are absolute win32 paths? Or, can you recommend a way to write tests for win32 mode? // it seems like this would _always_ truthy.
// on my mac, `pathModule === pathModule.posix` and `pathModule !== pathModule.win32`
// however, `!!pathModule.win32 === true`
if (pathModule.posix) {
const { posix } = pathModule;
sep = posix.sep;
relative = posix.relative;
} else {
// would this even happen? is `path.posix` simply not available on windows?
sep = pathModule.sep;
relative = pathModule.relative;
}
const isWin = process.platform === 'win32'; // apparently you can check if the path in question is an absolute path by platform
pathModule.win32.isAbsolute('C:\\') // --> true
pathModule.posix.isAbsolute('C:\\') // --> false |
So the Windows paths work correctly when Or is there a requirement in Webpack to treat Windows-like paths on UNIX-like systems as Windows paths? |
To write tests for Windows, my first try would be:
|
I think webpack works platform specific like you are assuming. I was initially trying to get things to work the way that I’ll separate the tests and see how it goes. Thanks!! |
|
Is it possible to put an easter egg to enable win32 mode for testing purposes? Like Going to try this: https://stackoverflow.com/questions/30405397/how-do-you-mock-process-platform-in-jasmine-tests Didn't work. The TypeScript gets compiled funky and uses a weird method of reading from process. Overriding process like that isn't going to work. Additionally, I need to use decache to remove memfs from the |
Hacked my local copy of volume. Still having issues. // hacked node_modules/memfs/lib/volume.js
var isWin
try {
if (process_1.default.env.NODE_ENV === 'test' && process_1.default.env.PATH_MODE === 'posix') {
isWin = false
} else if (process_1.default.env.NODE_ENV === 'test' && process_1.default.env.PATH_MODE === 'win32') {
isWin = true
} else {
isWin = process_1.default.platform === 'win32';
}
} catch (e) {}
console.log({isWin}) // --> true // test.js
process.env.NODE_ENV = 'test' // eslint-disable-line no-process-env
process.env.PATH_MODE = 'win32' // eslint-disable-line no-process-env
const decache = require('decache');
decache("memfs");
const {Volume} = require("memfs");
const fs = new Volume()
fs.mkdirpSync("C:\\test")
console.log(fs.toJSON())
// { '/Users/username/my-test/C:\\test': null } When I run my script I see: $ node test.js
{ isWin: true }
{ '/Users/username/my-test/C:\\test': null } |
Ok, more hacking. The problem is how
type TResolve = (filename: string, base?: string) => string;
let resolve: TResolve = (filename, base = process.cwd()) => resolveCrossPlatform(base, filename);
if (isWin) {
const _resolve = resolve;
// 1. unixify is checking for platform which make this function a no-op
const { unixify } = require('fs-monkey/lib/correctPath');
// 2. the base is posix and so is resolve so I actually need this:
// unixify(_resolve(unixify(filename), base))
resolve = (filename, base) => unixify(_resolve(filename, base));
} Conclusions:
// hack
if (isWin) {
var _resolve_1 = resolve;
// inline unixify
var unixify_1 = (filepath, stripTrailing = true) => {
if(isWin) {
// inlined this function too (and the rest of correctPath.js)
filepath = normalizePath(filepath, stripTrailing);
return filepath.replace(/^([a-zA-Z]+:|\.\/)/, '');
}
return filepath;
};
resolve = function (filename, base) {
// unixify the filename when resolve is posix
if (resolveCrossPlatform === pathModule.posix.resolve) {
return _resolve_1(unixify_1(filename), base);
}
// unixfy the result if we're really win32
return unixify_1(_resolve_1(filename, base));
};
} |
Actually, import * as memfsProcess from 'memfs/lib/process';
memfsProcess.default = memfsProcess.createProcess({});
memfsProcess.default.platform = 'win32';
import memfs from 'memfs';
// ... I think that allowing users to configure the |
Haven't had time to read this thread properly, I will do it later, but just wanted to mention one thing. Path handling in Node and So, how paths are handled depends on the const path = require('path');
Object.assign(path, path.win32);
Object.assign(path.posix, path.win32); Don't know if the above would actually work, maybe better to only replace specific methods used by |
I’m not well-versed at TypeScript (2019 goal) but I think I’ll take a swing at this and submit a PR. My thinking is that a relatively minor patch to the We’re really talking about a narrow set of edge cases that are all smoothed over by the resolve function. We simply need to make it aware of a few more situations.
Modes 3 and 4 would be called “strict” mode, and would be the default since that’s how it works today and it matches Setting a specific “posix/win32” mode would give the For instance, someone using memfs in their project could use all posix paths in their code and still have it work cross platform by setting mode to “posix”. Using “mixed” mode is what |
I also have a need for this. I have platform specific code that I should be able to test on any platform. I'm using vitest and I tried using:
but according to vitest-dev/vitest#1336 (comment) this won't work because memfs does not publish an ESM. If it did then I don't think I wouldn't need any specific behaviour in memfs to do this. I'm not sure about Jest, but I think it might be possible to do this. |
I'm thinking it would be good to do a soft reset on this issue, because it's not clear to me the actual problem (it sounds like there could be a few problems, but then we should have an issue for each ideally), and we've had some changes landed since this issue was originally opened which I think might help. Ultimately my current understanding is that I say this as a framing device: I could very well be wrong, in which case please open a new issue with as much information as possible, including code showing the behaviour of Now this isn't to say that we shouldn't go beyond what |
I just found this issue when I was debugging windows-path related stuff, and the goal here is simple: to write unit tests for windows-related path functionality without having to spin up an actual windows machine. Memfs is awesome for writing fs unit tests. It would be much more awesome if we could test all windows codepaths in the same breath! It would be a lifesaver for me, and I would be happy if I just got some guidance by some of the library's maintainers on what would be the best way to implement this. |
In that case I think we effectively want a sense of "mode" in As for the best way to implement it, there's a couple of ways that come to mind but as for which is the best one I don't know right now - so I'd probably start with the internal logic first since that should be more obvious i.e. we're going to want to ensure anytime we do anything with a path separator, it's drawing a single place (you could start with a function that just returns the hardcoded linux separator, that would mean we could easily add logic to have it return a different separator). |
webpack/webpack-dev-middleware#366 (comment)
memfs
does not handle win32 paths starting withC:\
(unless you are actually on a windows machine). Probably not on your roadmap but it's a blocker for a personal pipe dream of replacingmemory-fs
withmemfs
.By comparison,
memory-fs
treats paths starting with a/
as probably posix and starting with[A-Z]:
as probably win32. In the examples above, the presence ofC:
would be enough to convincememory-fs
that it was an absolute win32 path.memory-fs
has no support for relative paths and throws if it encounters one. The relative path compromise thatmemfs
makes is probably preferred as it's more similar to whatfs
does. But in the case of a win32 path prefix,memory-fs
makes the more flexible choice.FWIW, on my machine (a Mac), the following works exactly as
memfs
does. I don't know what it does on a Windows machine.The text was updated successfully, but these errors were encountered: