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

existsSync throws an error instead of returning false in Node 11 #256

Closed
nzakas opened this issue Nov 16, 2018 · 7 comments · Fixed by #260
Closed

existsSync throws an error instead of returning false in Node 11 #256

nzakas opened this issue Nov 16, 2018 · 7 comments · Fixed by #260

Comments

@nzakas
Copy link

nzakas commented Nov 16, 2018

Environment Info:

$ node --version
v11.2.0
$ npm --version
6.4.1
$ nvm --version
0.33.11

MockFS version: 4.7.0

We recently found one test using mock-fs that fails consistently in Node.js v11:
https://travis-ci.org/eslint/eslint/jobs/456094527#L484

This occurs because existsSync ends up throwing an error instead of returning false (which it does consistently in other Node.js versions).

The test in question looks like this:

describe("checkPackageJson()", () => {
    after(() => {
        mockFs.restore();
    });

    it("should return true if package.json exists", () => {
        mockFs({
            "package.json": "{ \"file\": \"contents\" }"
        });

        assert.strictEqual(npmUtils.checkPackageJson(), true);
    });

    it("should return false if package.json does not exist", () => {
        mockFs({});
        assert.strictEqual(npmUtils.checkPackageJson(), false);
    });
});

The first test passes while the second one throws an error in Node.js 11 (Node.js 6, 8, and 10 all pass). npmUtils.checkPackageJson() calls fs.existsSync under the covers.

@maxwellgerber
Copy link
Contributor

#254 (comment)
I'm not the maintainer for this library so take this with a grain of salt - I don't think mock-fs as it is currently implemented will ever support Node 11+.

not-an-aardvark added a commit to eslint/eslint that referenced this issue Nov 18, 2018
This is a workaround for tschaub/mock-fs#256. It seems like we might eventually need to find an alternative to mock-fs for future Node versions, but this workaround should make the build pass in the meantime.
aladdin-add pushed a commit to eslint/eslint that referenced this issue Nov 19, 2018
This is a workaround for tschaub/mock-fs#256. It seems like we might eventually need to find an alternative to mock-fs for future Node versions, but this workaround should make the build pass in the meantime.
@nzakas
Copy link
Author

nzakas commented Nov 19, 2018 via email

@mheggeseth
Copy link

This has found its way into (v10.14.2 LTS) independent of making process.binding() internal. existsSync was updated to use the synchronous version of process.binding("fs").access().

@jan-molak
Copy link

Same issue with node 10.14.2 - mock-fs fs.existsSync throws ENOENT instead of returning false

pirog added a commit to pirog/mock-fs that referenced this issue Jan 11, 2019
… to fs.existsSync introduced in later node 10 versions
@pirog
Copy link

pirog commented Jan 11, 2019

For those interested, currently using this as a workaround:
491fec8

No idea whether that is a good fix or not so YMMV

@tschaub
Copy link
Owner

tschaub commented Feb 4, 2019

Thanks for the report @nzakas. The 4.8.0 release addresses issues with Node 11 (and 10) compatibility (thanks @huochunpeng, see #260).

@nzakas
Copy link
Author

nzakas commented Feb 5, 2019

Awesome, thank you!

davidfirst added a commit to teambit/bit-javascript that referenced this issue Feb 6, 2019
Photonios pushed a commit to SectorLabs/js-lingui that referenced this issue Dec 9, 2019
cebjyre added a commit to KurtWagner/bitbucket-toolbox that referenced this issue Jun 2, 2021
Prior to 4.8.0 there was an issue with mock-fs' implementation of
existsSync throwing an exception under node 10+ instead of returning
false when a mocked file didn't exist. See tschaub/mock-fs#256
KurtWagner pushed a commit to KurtWagner/bitbucket-toolbox that referenced this issue Jun 3, 2021
* Update package.json to allow usage by modern node

Change the node engines to remove the upper limit

* Update mock-fs dependency

Prior to 4.8.0 there was an issue with mock-fs' implementation of
existsSync throwing an exception under node 10+ instead of returning
false when a mocked file didn't exist. See tschaub/mock-fs#256

* Add node 14 to the travis config
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 a pull request may close this issue.

6 participants