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

memfs breaks with fs-extra^10.0.0 #803

Closed
Phillip9587 opened this issue Jan 14, 2022 · 13 comments
Closed

memfs breaks with fs-extra^10.0.0 #803

Phillip9587 opened this issue Jan 14, 2022 · 13 comments

Comments

@Phillip9587
Copy link

memfs does not export fs.realpath.native. It is available since Node.js 9.2.0.
https://nodejs.org/api/fs.html#fsrealpathnativepath-options-callback

fs-extra uses univeralify to create a promise version of fs.realpath.native. Because memfs does not provide it, universalify throws a error:

TypeError: Cannot read properties of undefined (reading 'name')

      at Object.<anonymous>.exports.fromCallback (../../node_modules/fs-extra/node_modules/universalify/index.js:15:26)
      at Object.<anonymous> (../../node_modules/fs-extra/lib/fs/index.js:57:27)
      at Object.<anonymous> (../../node_modules/fs-extra/lib/index.js:5:6)

The error occurs even if realpath.native is not used.

The line of code:
https://github.com/jprichardson/node-fs-extra/blob/a84ef6dd8969f57d16e23267e1790def791e9a82/lib/fs/index.js#L57

@Phillip9587
Copy link
Author

This is related to jprichardson/node-fs-extra#887

@lamweili
Copy link

While jprichardson/node-fs-extra#953 might address this such that fs-extra doesn't fail on import, memfs should still properly export fs.realpath.native.

Putting in simply, without fs-extra, if one actually used fs.realpath.native in their code, they would still have a problem due to memfs (or its underlying dependencies).

@G-Rath
Copy link
Collaborator

G-Rath commented Apr 10, 2022

PRs are always welcome :)

@lamweili
Copy link

lamweili commented Apr 10, 2022

@G-Rath, assuming it's not a NodeJS <9.2.0 issue, I am unable to replicate it using NodeJS 14 with memfs@^3.4.1.

const assert = require('assert');
const memfs = require('memfs'); // memfs@^3.4.1

assert(typeof fs.realpath.native === 'function'); // is fine, not undefined

const fse = require('fs-extra'); // fs-extra@^10.0.1 - is fine, no errors as claimed

Might need more details from @Phillip9587.
It is also possible to be an inherent nx behaviour that altered fs but reported wrongly as nrwl/nx#8531.

@lamweili
Copy link

lamweili commented Apr 10, 2022

@Phillip9587, can you do a console.log(typeof fs.realpath.native) at the following 4 places?

  1. the very first line of code
  2. just before your nx import
    (I'm not familiar with this)
  3. just before your memfs import
  4. just before your fs-extra import

That would help to determine the underlying cause, if its environmental issues, or narrow down to a specific dependency (in-between) that has monkey-patched fs, causing fs-realpath.native to be undefined somewhat.

You might want to check the runtime NodeJS version through console.log(process.version).

@Phillip9587
Copy link
Author

@peteriman I'm gonna look into it on monday. Hope it was no wrong report from my side.

@lamweili
Copy link

@Phillip9587 Any findings so far?

Nevertheless, it is likely related to fs.realpath.native being undefined, causing import of fs-extra@10.0.0 to throw error.

Defensively patched in jprichardson/node-fs-extra#953 and released in fs-extra@10.1.0.

But still, why is fs.realpath.native undefined? That is a root cause to be investigated.

@G-Rath
Copy link
Collaborator

G-Rath commented May 17, 2022

Has anyone been able to reproduce this on Node 14 or 16, using the latest version?

@lamweili
Copy link

I am unable to replicate it using NodeJS 14 with memfs@3.4.1.

// pre-req
console.log(fs.realpath.native);
> [Function (anonymous)]

// memfs test
const memfs = require('memfs');
console.log(fs.realpath.native);
> [Function (anonymous)]

My suspicion is with nx (or other dependencies that altered fs).
It might not be a memfs issue to begin with. Would need clarification from @Phillip9587.

@Phillip9587
Copy link
Author

@peteriman Sorry for my late reply. The error originally ocurred after updating from fs-extra 9 to 10 in the Nx repo. The unit tests which used memfs were failing due to the error above. I looked into it and nx does not patch any fs methods so it was definitly related to memfs or/and fs-extra. But as you said the fs-extra 10.1.0 release fixes my issue ....

I think your test is wrong. I tested on Nodejs 16.15.0 and got the following:

// pre-req
const nodefs = require('fs');
console.log(nodefs.realpath.native);
> [Function (anonymous)]

// memfs test
const { fs } = require('memfs');
console.log(fs.realpath.native);
> undefined

I don't have any time to investigate further I'm sorry....

@Phillip9587
Copy link
Author

@peteriman memfs 3.4.1

@lamweili
Copy link

lamweili commented May 18, 2022

@Phillip9587 Ah, thanks for that!
I didn't assign it to the fs variable in my code, my bad!


@G-Rath I tested @Phillip9587's code snippet. It happens on memfs@3.4.1 - 3.4.3 with NodeJS 14 and 16.

If memfs is imported to the fs variable name (as in the readme), it does not export fs.realpath.native:

// pre-req
const nodefs = require('fs');
console.log(nodefs.realpath.native);
> [Function (anonymous)]

// memfs test
const { fs } = require('memfs');
console.log(fs.realpath.native);
> undefined

Might be related to the under-the-hood dependency, fs-monkey.

@Phillip9587
Copy link
Author

Defensively patched in jprichardson/node-fs-extra#953 and released in fs-extra@10.1.0.

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

3 participants