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: add support for compileFunction allowing us to avoid the module wrapper #9252

Merged
merged 5 commits into from Dec 2, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -17,15 +17,18 @@
- `[jest-diff]` Add `changeColor` and `patchColor` options ([#8911](https://github.com/facebook/jest/pull/8911))
- `[jest-diff]` Add `trailingSpaceFormatter` option and replace cyan with `commonColor` ([#8927](https://github.com/facebook/jest/pull/8927))
- `[jest-diff]` Add `firstOrLastEmptyLineReplacement` option and export 3 `diffLines` functions ([#8955](https://github.com/facebook/jest/pull/8955))
- `[jest-environment]` Add optional `compileFunction` next to `runScript` ([#9252](https://github.com/facebook/jest/pull/9252))
- `[jest-environment-jsdom]` Add `fakeTimersLolex` ([#8925](https://github.com/facebook/jest/pull/8925))
- `[jest-environment-node]` Add `fakeTimersLolex` ([#8925](https://github.com/facebook/jest/pull/8925))
- `[jest-environment-node]` Add `queueMicrotask` ([#9140](https://github.com/facebook/jest/pull/9140))
- `[jest-environment-node]` Implement `compileFunction` ([#9140](https://github.com/facebook/jest/pull/9140))
- `[@jest/fake-timers]` Add Lolex as implementation of fake timers ([#8897](https://github.com/facebook/jest/pull/8897))
- `[jest-get-type]` Add `BigInt` support. ([#8382](https://github.com/facebook/jest/pull/8382))
- `[jest-matcher-utils]` Add `BigInt` support to `ensureNumbers` `ensureActualIsNumber`, `ensureExpectedIsNumber` ([#8382](https://github.com/facebook/jest/pull/8382))
- `[jest-reporters]` Export utils for path formatting ([#9162](https://github.com/facebook/jest/pull/9162))
- `[jest-runner]` Warn if a worker had to be force exited ([#8206](https://github.com/facebook/jest/pull/8206))
- `[jest-runtime]` [**BREAKING**] Do not export `ScriptTransformer` - it can be imported from `@jest/transform` instead ([#9256](https://github.com/facebook/jest/pull/9256))
- `[jest-runtime]` Use `JestEnvironment.compileFunction` if available to avoid the module wrapper ([#9252](https://github.com/facebook/jest/pull/9252))
- `[jest-snapshot]` Display change counts in annotation lines ([#8982](https://github.com/facebook/jest/pull/8982))
- `[jest-snapshot]` [**BREAKING**] Improve report when the matcher has properties ([#9104](https://github.com/facebook/jest/pull/9104))
- `[jest-snapshot]` Improve colors when snapshots are updatable ([#9132](https://github.com/facebook/jest/pull/9132))
Expand Down
14 changes: 0 additions & 14 deletions e2e/__tests__/__snapshots__/globals.test.ts.snap
Expand Up @@ -18,20 +18,6 @@ Ran all test suites.
`;

exports[`cannot have describe with no implementation 1`] = `
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run

Missing second argument. It must be a callback function.

1 |
> 2 | describe('describe, no implementation');
| ^
3 |

at Object.<anonymous> (__tests__/onlyConstructs.test.js:2:14)
`;

exports[`cannot have describe with no implementation 2`] = `
Test Suites: 1 failed, 1 total
Tests: 0 total
Snapshots: 0 total
Expand Down
24 changes: 15 additions & 9 deletions e2e/__tests__/errorOnDeprecated.test.ts
Expand Up @@ -43,18 +43,24 @@ testFiles.forEach(testFile => {
expect(result.exitCode).toBe(1);
let {rest} = extractSummary(result.stderr);

if (
nodeMajorVersion < 12 &&
testFile === 'defaultTimeoutInterval.test.js'
) {
if (testFile === 'defaultTimeoutInterval.test.js') {
const lineEntry = '(__tests__/defaultTimeoutInterval.test.js:10:3)';

expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
if (nodeMajorVersion < 10) {
expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);

rest = rest.replace(
`at Object.<anonymous>.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
rest = rest.replace(
`at Object.<anonymous>.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
} else if (nodeMajorVersion < 12) {
expect(rest).toContain(`at Object.test ${lineEntry}`);

rest = rest.replace(
`at Object.test ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
}
}

expect(wrap(rest)).toMatchSnapshot();
Expand Down
11 changes: 10 additions & 1 deletion e2e/__tests__/failures.test.ts
Expand Up @@ -39,7 +39,7 @@ test('not throwing Error objects', () => {
expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
stderr = runJest(dir, ['duringTests.test.js']).stderr;

if (nodeMajorVersion < 12) {
if (nodeMajorVersion < 10) {
const lineEntry = '(__tests__/duringTests.test.js:38:8)';

expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);
Expand All @@ -48,6 +48,15 @@ test('not throwing Error objects', () => {
`at Object.<anonymous>.done ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
} else if (nodeMajorVersion < 12) {
const lineEntry = '(__tests__/duringTests.test.js:38:8)';

expect(stderr).toContain(`at Object.done ${lineEntry}`);

stderr = stderr.replace(
`at Object.done ${lineEntry}`,
`at Object.<anonymous> ${lineEntry}`,
);
}

expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
Expand Down
42 changes: 41 additions & 1 deletion e2e/__tests__/globals.test.ts
Expand Up @@ -7,6 +7,7 @@

import * as path from 'path';
import {tmpdir} from 'os';
import {compileFunction} from 'vm';
import {wrap} from 'jest-snapshot-serializer-raw';
import runJest from '../runJest';
import {
Expand Down Expand Up @@ -125,7 +126,46 @@ test('cannot have describe with no implementation', () => {

const rest = cleanStderr(stderr);
const {summary} = extractSummary(stderr);
expect(wrap(rest)).toMatchSnapshot();

const rightTrimmedRest = rest
.split('\n')
.map(l => l.trimRight())
.join('\n')
.trim();

if (typeof compileFunction === 'function') {
expect(rightTrimmedRest).toEqual(
`
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run

Missing second argument. It must be a callback function.

1 |
> 2 | describe('describe, no implementation');
| ^
3 |

at Object.describe (__tests__/onlyConstructs.test.js:2:5)
`.trim(),
);
} else {
expect(rightTrimmedRest).toEqual(
`
FAIL __tests__/onlyConstructs.test.js
● Test suite failed to run

Missing second argument. It must be a callback function.

1 |
> 2 | describe('describe, no implementation');
| ^
3 |

at Object.<anonymous> (__tests__/onlyConstructs.test.js:2:14)
`.trim(),
);
}
expect(wrap(summary)).toMatchSnapshot();
});

Expand Down
24 changes: 23 additions & 1 deletion packages/jest-environment-node/src/index.ts
Expand Up @@ -5,7 +5,13 @@
* LICENSE file in the root directory of this source tree.
*/

import {Context, Script, createContext, runInContext} from 'vm';
import {
Context,
Script,
compileFunction,
createContext,
runInContext,
} from 'vm';
import {Config, Global} from '@jest/types';
import {ModuleMocker} from 'jest-mock';
import {installCommonGlobals} from 'jest-util';
Expand Down Expand Up @@ -110,6 +116,22 @@ class NodeEnvironment implements JestEnvironment {
}
return null;
}

compileFunction(code: string, params: Array<string>, filename: string) {
if (this.context) {
return compileFunction(code, params, {
filename,
parsingContext: this.context,
}) as any;
Copy link
Member Author

Choose a reason for hiding this comment

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

it returns Function

}
return null;
}
}

// `jest-runtime` checks for `compileFunction`, so this makes sure to not expose that function if it's unsupported by this version of node
// Should be removed when we drop support for node 8
if (typeof compileFunction !== 'function') {
delete NodeEnvironment.prototype.compileFunction;
SimenB marked this conversation as resolved.
Show resolved Hide resolved
}

export = NodeEnvironment;
5 changes: 5 additions & 0 deletions packages/jest-environment/src/index.ts
Expand Up @@ -44,6 +44,11 @@ export declare class JestEnvironment {
fakeTimersLolex: LolexFakeTimers | null;
moduleMocker: jestMock.ModuleMocker | null;
runScript<T = unknown>(script: Script): T | null;
compileFunction?<T = unknown>(
Copy link
Member Author

Choose a reason for hiding this comment

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

I've made it optional so we're not blocked by jsdom having support. It'll silently use it if it's ever implemented

code: string,
params: Array<string>,
filename: string,
): T | null;
setup(): Promise<void>;
teardown(): Promise<void>;
handleTestEvent?(event: Circus.Event, state: Circus.State): void;
Expand Down
53 changes: 39 additions & 14 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -730,20 +730,41 @@ class Runtime {
}
}

const script = this.createScriptFromCode(transformedFile.code, filename);
let compiledFunction: ModuleWrapper | null;

if (typeof this._environment.compileFunction === 'function') {
try {
compiledFunction = this._environment.compileFunction<ModuleWrapper>(
transformedFile.code,
this.constructInjectedModuleParameters(),
filename,
);
} catch (e) {
throw handlePotentialSyntaxError(e);
}
} else {
const script = this.createScriptFromCode(transformedFile.code, filename);

const runScript = this._environment.runScript<RunScriptEvalResult>(script);
const runScript = this._environment.runScript<RunScriptEvalResult>(
script,
);

if (runScript === null) {
if (runScript === null) {
compiledFunction = null;
} else {
compiledFunction = runScript[EVAL_RESULT_VARIABLE];
}
}

if (compiledFunction === null) {
this._logFormattedReferenceError(
'You are trying to `import` a file after the Jest environment has been torn down.',
);
process.exitCode = 1;
return;
}

//Wrapper
runScript[EVAL_RESULT_VARIABLE].call(
compiledFunction.call(
localModule.exports,
localModule as NodeModule, // module object
localModule.exports, // module exports
Expand Down Expand Up @@ -1107,7 +1128,19 @@ class Runtime {
}

private wrapCodeInModuleWrapper(content: string) {
const args = [
const args = this.constructInjectedModuleParameters();

return (
'({"' +
EVAL_RESULT_VARIABLE +
`":function(${args.join(',')}){` +
content +
'\n}});'
);
}

private constructInjectedModuleParameters() {
return [
'module',
'exports',
'require',
Expand All @@ -1117,14 +1150,6 @@ class Runtime {
'jest',
...this._config.extraGlobals,
];

return (
'({"' +
EVAL_RESULT_VARIABLE +
`":function(${args.join(',')}){` +
content +
'\n}});'
);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/jest-transform/src/enhanceUnexpectedTokenMessage.ts
Expand Up @@ -17,7 +17,8 @@ export default function handlePotentialSyntaxError(
}

if (
e instanceof SyntaxError &&
// `instanceof` might come from the wrong context
e.name === 'SyntaxError' &&
Copy link
Member Author

Choose a reason for hiding this comment

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

the SyntaxError thrown by compileFunction comes from the passed in context

(e.message.includes('Unexpected token') ||
e.message.includes('Cannot use import')) &&
!e.message.includes(' expected')
Expand Down