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

Unsorted Filenames #37

Closed
wandersonwhcr opened this issue Sep 9, 2017 · 8 comments
Closed

Unsorted Filenames #37

wandersonwhcr opened this issue Sep 9, 2017 · 8 comments

Comments

@wandersonwhcr
Copy link

wandersonwhcr commented Sep 9, 2017

Hello!

First, thank you for your package. I am a Node.js beginner and memfs helps me a lot with testing and mocking.

I am developing a package to load configuration alphabetically by filenames. Searching for Node.js fs readdir behavior, I just realize this method is always sorted on Linux, but not on Windows (or filesystems, whatever).

nodejs/node#3232

https://nodejs.org/api/fs.html#fs_fs_readdir_path_options_callback
http://man7.org/linux/man-pages/man3/readdir.3.html#NOTES

Thinking about it, currently, I can't simulate a readdir with unsorted filenames in memfs.

https://github.com/streamich/memfs/blob/master/src/volume.ts#L1486

Can we improve some configuration to disable that feature, or implement another sort?

@wandersonwhcr
Copy link
Author

Related with wandersonwhcr/node-config#25

@streamich
Copy link
Owner

streamich commented Sep 9, 2017

Well, in general the file names are not guaranteed to be sorted, as readdir(3) states:

The order in which filenames are read by successive calls to readdir() depends on the filesystem implementation;

I added sorting to readdir function because Node's fs.readdir would return files sorted on my Ubuntu machine, I guess that was a mistake.

From nodejs/node#3232 I see that Linux users will expect the files to be sorted. So, I will add a check for Windows:

if(!isWin && encoding !== 'buffer') list.sort();

@wandersonwhcr
Copy link
Author

But if I want to simulate Windows runtime in my tests, how can I do that?

I don't know if memfs must do this, but it will improve tests on dependent packages.

@streamich
Copy link
Owner

streamich commented Sep 9, 2017

But if I want to simulate Windows runtime in my tests, how can I do that?

process.platform = 'win32';

https://github.com/streamich/memfs/blob/master/src/volume.ts#L14

streamich added a commit that referenced this issue Sep 10, 2017
@streamich
Copy link
Owner

Published under version 2.5.4

@wandersonwhcr
Copy link
Author

Thanks for your attention, but I think it's wrong to modify a global element like process to run a test. I'll try my own way. Again, thank you 👍

@streamich
Copy link
Owner

@wandersonwhcr how would you suggest to do it?

@wandersonwhcr
Copy link
Author

I'll create a PR soon

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

No branches or pull requests

2 participants