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

feat: upgrade to JSDOM 16 #9606

Merged
merged 19 commits into from May 2, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -29,6 +29,7 @@
- `[jest-transform]` Correct sourcemap behavior for transformed and instrumented code ([#9460](https://github.com/facebook/jest/pull/9460))
- `[jest-transform]` Allow instrumentation of transformed files with weird file extensions ([#9589](https://github.com/facebook/jest/pull/9589))
- `[pretty-format]` Export `OldPlugin` type ([#9491](https://github.com/facebook/jest/pull/9491))
- `[jest-environment-jsdom]` Upgrade `jsdom` v16 to fix memory leak ([#9606](https://github.com/facebook/jest/pull/9606))

### Chore & Maintenance

Expand Down
28 changes: 28 additions & 0 deletions e2e/__tests__/jestEnvironmentJsdom.test.ts
@@ -0,0 +1,28 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import * as path from 'path';
import {tmpdir} from 'os';
import {cleanup, writeFiles} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(tmpdir(), 'jest_environment_jsdom_test');

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

test('check is not leaking memory', () => {
writeFiles(DIR, {
'__tests__/a.test.js': `test('a', () => console.log('a'));`,
'__tests__/b.test.js': `test('b', () => console.log('b'));`,
'package.json': JSON.stringify({jest: {testEnvironment: 'jsdom'}}),
});

const {stderr} = runJest(DIR, ['--detect-leaks', '--runInBand']);
expect(stderr).toMatch(/PASS\s__tests__\/a.test.js/);
expect(stderr).toMatch(/PASS\s__tests__\/b.test.js/);
});
4 changes: 2 additions & 2 deletions packages/jest-environment-jsdom/package.json
Expand Up @@ -15,10 +15,10 @@
"@jest/types": "^25.1.0",
"jest-mock": "^25.1.0",
"jest-util": "^25.1.0",
"jsdom": "^15.1.1"
"jsdom": "^16.2.0"
lh0x00 marked this conversation as resolved.
Show resolved Hide resolved
},
"devDependencies": {
"@types/jsdom": "^12.2.4"
"@types/jsdom": "^16.1.0"
},
"engines": {
"node": ">= 8.3"
Expand Down
21 changes: 17 additions & 4 deletions packages/jest-environment-jsdom/src/index.ts
Expand Up @@ -14,7 +14,7 @@ import {
LolexFakeTimers,
} from '@jest/fake-timers';
import {EnvironmentContext, JestEnvironment} from '@jest/environment';
import {JSDOM, VirtualConsole} from 'jsdom';
import {DOMWindow, JSDOM, VirtualConsole} from 'jsdom';

// The `Window` interface does not have an `Error.stackTraceLimit` property, but
// `JSDOMEnvironment` assumes it is there.
Expand All @@ -27,6 +27,7 @@ type Win = Window &

class JSDOMEnvironment implements JestEnvironment {
dom: JSDOM | null;
vm: DOMWindow | null;
lh0x00 marked this conversation as resolved.
Show resolved Hide resolved
fakeTimers: LegacyFakeTimers<number> | null;
fakeTimersLolex: LolexFakeTimers | null;
global: Win;
Expand All @@ -41,12 +42,18 @@ class JSDOMEnvironment implements JestEnvironment {
virtualConsole: new VirtualConsole().sendTo(options.console || console),
...config.testEnvironmentOptions,
});
const global = (this.global = this.dom.window.document.defaultView as Win);

this.vm = this.dom.getInternalVMContext();

const global = (this.global = this.dom.window.document.defaultView as Win);
if (!global) {
throw new Error('JSDOM did not return a Window object');
}

// In the `jsdom@16`, ArrayBuffer was not added to Window, ref: https://github.com/jsdom/jsdom/commit/3a4fd6258e6b13e9cf8341ddba60a06b9b5c7b5b
// Install ArrayBuffer to Window to fix it. Make sure the test is passed, ref: https://github.com/facebook/jest/pull/7626
global.ArrayBuffer = ArrayBuffer;
Comment on lines +50 to +52
Copy link
Contributor

@ExE-Boss ExE-Boss Nov 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is wrong, since it causes the newly‑created global to have the ArrayBuffer constructor from the outer realm, whereas jsdom/jsdom@3a4fd62 made it so that the inner realm has its own ArrayBuffer constructor and prototype chain.


Refs: #10786, #10854

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ExE-Boss great! Wanna send a PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Node's error-message stack size is limited at 10, but it's pretty useful
// to see more than that when a test fails.
this.global.Error.stackTraceLimit = 100;
Expand Down Expand Up @@ -120,16 +127,22 @@ class JSDOMEnvironment implements JestEnvironment {
// @ts-ignore
this.global = null;
this.dom = null;
this.vm = null;
this.fakeTimers = null;
this.fakeTimersLolex = null;
}

runScript<T = unknown>(script: Script): T | null {
if (this.dom) {
return this.dom.runVMScript(script) as any;
if (this.vm) {
return script.runInContext(this.vm);
}

return null;
}

getVmContext() {
return this.vm;
}
}

export = JSDOMEnvironment;