Mocking of fs module in Jest broken in 10.5.0

[SimenB]

• Version: v10.5.0
• Platform: Darwin Simens-MBP 17.4.0 Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64 x86_64
• Subsystem: fs

With the following file:

jest.mock('fs');

const mockedFs = require('fs');

console.log(typeof mockedFs.ReadStream);
console.log(typeof mockedFs.WriteStream);

test('empty test to have Jest run the file', () => {
  expect(true).toBe(true);
});

It prints the following in Node 10.4.1:

  ● Console

    console.log test.js:5
      function
    console.log test.js:6
      function

And the following in Node 10.5.0:

  ● Console

    console.log test.js:5
      undefined
    console.log test.js:6
      undefined

More concretely, the following is missing (File){Read,Write}Stream in 10.5.0, while they're present in 10.4.1:

const fs = require('fs');
const jestMock = require('jest-mock');

const {members: mockMembers} = jestMock.getMetadata(fs);

console.log(mockMembers);

Source of that function: https://github.com/facebook/jest/blob/24df0751a818e570fe74dd0d73285a63e322d37f/packages/jest-mock/src/index.js#L642-L723

Probably related to #20764.

I'll add that this should probably be addressed in Jest, but this is potentially pretty breaking for consumers of Jest (of which not everybody will upgrade to the latest version).

In particular, graceful-fs completely breaks if required after a jest.mock('fs');

jest.mock('fs');

require('graceful-fs');

Explodes with

    TypeError: Cannot read property 'prototype' of undefined

      1 | jest.mock('fs');
      2 |
    > 3 | require('graceful-fs');
      4 |

      at patch (node_modules/graceful-fs/graceful-fs.js:166:54)
      at Object.<anonymous> (ugh.test.js:3:1)

[vitkarpov]

Seems in v10.5.0 ReadStream and WriteStream became getters with lazy loading instead of regular properties.

In Jest getSlots method passes getters, that's why this props are not included eventually. @SimenB Probably you know the reason why getters not included in the first place.

[SimenB]

@jasnell Sorry to ping you directly, but are there any plans for a fix here?

[jasnell]

@SimenB ... my apologies for just seeing this, I've been on vacation and have a serious backlog of issues to catch up on. It appears that @targos has a potential fix in the works.

[SimenB]

Dug into why we don't include accessors, and it seems to be by design: jestjs/jest#390

I'll try to ask if it's still needed

[targos]

Our accessors are enumerable:

> Object.getOwnPropertyDescriptor(fs, 'ReadStream')
{ get: [Function: get ReadStream],
  set: [Function: set ReadStream],
  enumerable: true,
  configurable: true }

[SimenB]

Yeah, the PR is poorly named. If you look at the code it uses Object.getOwnPropertyDescriptor and bails if it's a getter, it doesn't actually care about enumerable if it's a getter, which isn't mentioned.

[vitkarpov]

And now enumerability check is gone but it was there in the first place 🤔 Seems like unexpected disappearing to me.

[SimenB]

Good point, it would've worked if we'd have kept the enumerable check, I read that too quickly...

[SimenB]

jestjs/jest#409, removed without any context provided... I've asked, but haven't received a response yet

[vitkarpov]

@SimenB I assume you've asked within some private channel, could you, please, provide an update here? I'm intrigued 😃

[targos]

The breaking change is reverted on Node 10 (will be released with the next version)