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

erroneous ArrayBuffer polyfill when using node environment with node 7+ #3186

Closed
mbroadst opened this issue Mar 22, 2017 · 12 comments
Closed
Labels

Comments

@mbroadst
Copy link

Do you want to request a feature or report a bug?
bug

What is the current behavior?
When working with ArrayBuffers, it appears that when jest is run using the node environment (--env node) a polyfill is being used for ArrayBuffer incorrectly only in node v7+. It is unclear to me if this really is a jest-specific problem, but it does seem to be related to the jest environment.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

https://github.com/mbroadst/jest-arraybuffer-issue

  • switch to node v6.2.2, yarn install && yarn test and the test will pass
  • switch to node v7.74 (latest) yarn install && yarn test and the test will fail

What is the expected behavior?
the test passes

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.
see: github repo

@aaronabramov
Copy link
Contributor

i haven't looked deep into the issue, but what i found interesting is:

~/tmp/jest-arraybuffer-issue > yarn test
 FAIL  ./test.js
  ● fetch › should fetch a document

    expect(value).toBeInstanceOf(constructor)

    Expected value to be an instance of:
      "ArrayBuffer"
    Received:
      []
    Constructor:
      "ArrayBuffer"

      at test.js:8:26
      at process._tickCallback (internal/process/next_tick.js:109:7)

  fetch
    ✕ should fetch a document (372ms)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 total
Snapshots:   0 total
Time:        0.531s, estimated 1s
Ran all test suites.

and right after that

dabramov-mbp ~/tmp/jest-arraybuffer-issue master* ./node_modules/.bin/jest
 PASS  ./test.js
  fetch
    ✓ should fetch a document (398ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        1.136s
Ran all test suites.

@aaronabramov
Copy link
Contributor

ok. sorry, i missed --env node, now i can reproduce

@aaronabramov
Copy link
Contributor

it consistently fails on all node versions that i tried, even without Jest.

dabramov-mbp ~/tmp/jest-arraybuffer-issue master* nvm use 6.2.2
Now using node v6.2.2 (npm v3.9.5)
dabramov-mbp ~/tmp/jest-arraybuffer-issue master* node
> require('isomorphic-fetch'); fetch('https://google.com').then(response => response.buffer()).then(buf => console.log(buf.constructor.name));
Promise { <pending> }
> Buffer

This is probably related to this change:
node-fetch/node-fetch@f0d0675

@mbroadst
Copy link
Author

@DmitriiAbramov hm you're right that all of a sudden I can't repro the success case with earlier version of node - I'll work on updating that. In the meantime, your test is incorrect above, you need to check the constructor name of the buf.buffer (internal buffer) of the response:

mbroadst@gorgor:~/Development/jest-arraybuffer-issue (master %)$ node --version
v6.2.2
mbroadst@gorgor:~/Development/jest-arraybuffer-issue (master %)$ node
> require('isomorphic-fetch');
[Function]
>  fetch('https://google.com').then(r => r.buffer()).then(b => console.log('ctor: ', b.buffer.constructor.name, ', instanceOf: ', b.buffer instanceof ArrayBuffer));
Promise { <pending> }
> ctor:  ArrayBuffer , instanceOf:  true
mbroadst@gorgor:~/Development/jest-arraybuffer-issue (master %)$ node --version
v7.7.4
mbroadst@gorgor:~/Development/jest-arraybuffer-issue (master %)$ node
> require('isomorphic-fetch');
[Function]
>  fetch('https://google.com').then(r => r.buffer()).then(b => console.log('ctor: ', b.buffer.constructor.name, ', instanceOf: ', b.buffer instanceof ArrayBuffer));
Promise { <pending> }
> ctor:  ArrayBuffer , instanceOf:  true

So what we are seeing here is that in both versions of node the returned buffer has an internal ArrayBuffer which is an instanceof ArrayBuffer. What the broken jest test is showing you is that for some reason, when --env node is supplied to jest, that same object is not an instanceof ArrayBuffer, and so my guess is that ArrayBuffer is being polyfilled somewhere - but only when --env node is specified.

@mbroadst
Copy link
Author

@DmitriiAbramov the problem seems to be with the setup of your node environment here: https://github.com/facebook/jest/blob/master/packages/jest-environment-node/src/index.js#L29-L38

Specifically, I think this line:

const global = (this.global = vm.runInContext('this', this.context));

thinks its making a copy of the global node context, but it isn't at all (if you run that locally you'll see the result is {}). So if below that I put: global.ArrayBuffer = ArrayBuffer then this bug goes away. I'm not sure what the intent is here, so I'm refraining from submitting a PR, if you can fill me in I have no problem doing the work.

For instance, why not just say const global = Object.assign({}, global) ? Then you wouldn't have to manually assign the global properties.

@aaronabramov
Copy link
Contributor

ah. i know what's going on there.
Because we run tests in a separate isolated context (inner scope) we end up with two instances of ArrayBuffer (one from the inner scope, and one from the outer scope, where the Jest runner runs itself).
Both are exactly the same, both have constructor.name === 'ArrayBuffer', but they come from different node environments so they are two different objects and instanceof will return false.

The change that you suggested would fix the issue, since the it will share the same object between two environments.

We don't share all properties between outer and inner context, because if a test modifies something shared like Array.prototype.toString = () => 'lol' that will affect other test files (and that's really bad).

one thing i'm not sure is why fetch returns an instance of ArrayBuffer from the outer scope, rather than from the inner scope of the test.

@aaronabramov aaronabramov reopened this Mar 23, 2017
@aaronabramov
Copy link
Contributor

ok. i'm not sure what exactly is going on, but it seems like the Buffer that's created here
https://github.com/bitinn/node-fetch/blob/f0d0675d00845c59ac4bac58d8c9b89d2a39d38e/src/body.js#L269
is an instance of an outer scope Buffer and not inner scope. that's why the check fails

@mbroadst
Copy link
Author

@DmitriiAbramov are you sure? That should mean I could explicitly set global.Buffer = Buffer in the env setup and it should work, but it doesn't. Only setting global.ArrayBuffer = ArrayBuffer seems to work.

@tristan-shelton
Copy link

tristan-shelton commented Jul 26, 2018

I encountered this issue when writing some integration tests using the ws library in the node environment.

I set socket.binaryType = 'arraybuffer' and received an ArrayBuffer in the callback but it was failing instanceof ArrayBuffer checks.

Creating and using a custom environment solved it, code below:

const NodeEnvironment = require('jest-environment-node')

module.exports = class ArrayBufferEnvironment extends NodeEnvironment {
  constructor(config) {
    super(config)
    this.global.ArrayBuffer = ArrayBuffer
  }
}

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 26, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants