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

creates undefined directories #325

Closed
heygrady opened this issue Feb 9, 2019 · 5 comments · Fixed by #326
Closed

creates undefined directories #325

heygrady opened this issue Feb 9, 2019 · 5 comments · Fixed by #326

Comments

@heygrady
Copy link

heygrady commented Feb 9, 2019

Was comparing memfs and memory-fs and was failing a test. Discovered that memfs will create undefined directories when you fs.mkdirSync("/"); and that directory already exists.

// test.js
const {Volume} = require("memfs");

var fs = new Volume();

// snippet from https://github.com/webpack/memory-fs/blob/master/test/MemoryFileSystem.js#L18
fs.mkdirSync("/test");
console.log(fs.readdirSync("/"));
fs.mkdirSync("/test//sub/");
console.log(fs.readdirSync("/"));
fs.mkdirpSync("/test/sub2");
console.log(fs.readdirSync("/"));
fs.mkdirSync("/root\\dir");
console.log(fs.readdirSync("/"));
fs.mkdirpSync("/");
console.log(fs.readdirSync("/"));
fs.mkdirSync("/"); // <-- this should probably throw; creates an "undefined" folder instead
console.log(fs.readdirSync("/")); // --> [ 'root\\dir', 'test', 'undefined' ]
$ node test.js
[ 'test' ]
[ 'test' ]
[ 'test' ]
[ 'root\\dir', 'test' ]
[ 'root\\dir', 'test' ]
[ 'root\\dir', 'test', 'undefined' ]
@streamich streamich added the bug label Feb 9, 2019
@heygrady
Copy link
Author

heygrady commented Feb 9, 2019

I created a fork of memory-fs and tried to replace it with Volume. It's mostly working but some tests are failing. Most of the test failures are related to streams.

https://github.com/heygrady/memory-fs/tree/memfs

There are also some failures related to windows-style paths. I think those might be due to memory-fs using a normalize function on all paths and I didn't wrap the memfs methods to do that. (#327)

I'm exploring easy ways to make memory-fs API-compliant to work with webpack-dev-server.

webpack/webpack-dev-middleware#366

streamich added a commit that referenced this issue Feb 9, 2019
streamich pushed a commit that referenced this issue Feb 9, 2019
## [2.15.1](v2.15.0...v2.15.1) (2019-02-09)

### Bug Fixes

* 🐛 show directory path when throwing EISDIR in mkdir ([9dc7007](9dc7007))
* 🐛 throw when creating root directory ([f77fa8b](f77fa8b)), closes [#325](#325)
@streamich
Copy link
Owner

Thanks for reporting this! If you have examples of bugs related to streams, I would kindly ask you to open a new issue.

@streamich
Copy link
Owner

🎉 This issue has been resolved in version 2.15.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@heygrady
Copy link
Author

The streams issue was a false alarm. Turns out that streams in memory-fs don't work the same as fs and memfs. I think memfs has a more-correct implementation and I corrected the tests in my branch to follow the memfs/fs way.

@streamich
Copy link
Owner

Im memfs stream implementation is copied from Node almost verbatim.

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

Successfully merging a pull request may close this issue.

2 participants