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

Implemented temp storage through fs-mock #47

Closed
wants to merge 2 commits into from

Conversation

challet
Copy link

@challet challet commented Dec 11, 2019

Trying to implement the feature as spoken about on #15 (non persistent storage)
Seems to work great except when many instances of LocalStorage are run simultaneously, see this part of the tests.

By the way, I wanted to illustrate the confusion between the two storage types, but I'm now realizing two things :

  • The same path (os.tmpdir()) is used for both of them, and there would also be a mess with two instances using the the same path on the actual file system. edit: same path case removed.
  • Once a LocalStorage is instanciated without a path and mock-fs has taken over, the whole fs module would be affected. Also on other parts of the application.

@lmaccherone
Copy link
Owner

Hi @challet,

Thanks for taking this on. Were you going to fix the failing tests? I was holding off on looking at it until you had it passing testing. It looks like all that is failing is that it returns an error message rather than throwing an error.

Thanks,
Larry

@challet
Copy link
Author

challet commented Dec 16, 2019

Hi @lmaccherone, thanks for your reply. I wish I could fix the tests but they are here to illustrate that two instances, one of each type, cannot co-exist at the same time, due to how mock-fsis working . So it now depends on how we see this :

  • If it is a requirement to make them able to co-exist, I believe mock-fs is a dead-end, as it globally plugs a new behavior in into fs. Unless a way is found to isolate it from the global fs module.
  • If we see no reason to use one "real fs" instance along a "memory fs" instance and we acknowledge that fs is globally changed (with a warning somewhere in the documentation), then the failing test are irrelevant and can be removed.

Looking forward your opinion on this. I'm going to have a look at a way to isolate mock-fs from the global fs which I don't have a strong hope on.

@challet
Copy link
Author

challet commented Jan 7, 2020

Some update here after exploring the mock-fs repository :

  • The last major version removed a function that was meant for the usage we're aiming. (see this README section)

The mock.fs() function has been removed. This returned an object with fs-like methods without overriding the built-in fs module.

  • Downgrading the dependency to v3 would make it available back (see function doc in v3 README)
  • But It appears it is only compatible until Node 7.x (see mock-fs v3 initialisation code)

There is also an open issue with a close-related subject : tschaub/mock-fs#142

@lmaccherone
Copy link
Owner

Closing for inactivity

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.

2 participants