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 1 commit
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 ro `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
22 changes: 21 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,20 @@ 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;
}
}

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