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

fix: do not transform internally imported files #9900

Merged
merged 3 commits into from Apr 28, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -23,6 +23,7 @@
- `[jest-resolve]` Update `resolve` to a version using native `realpath`, which is faster than the default JS implementation ([#9872](https://github.com/facebook/jest/pull/9872))
- `[jest-resolve]` Pass custom cached `realpath` function to `resolve` ([#9873](https://github.com/facebook/jest/pull/9873))
- `[jest-runtime]` Add `teardown` method to clear any caches when tests complete ([#9906](https://github.com/facebook/jest/pull/9906))
- `[jest-runtime]` Do not pass files required internally through transformation when loading them ([#9900](https://github.com/facebook/jest/pull/9900))

## 25.4.0

Expand Down
59 changes: 37 additions & 22 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -343,13 +343,13 @@ class Runtime {
return core;
}

const transformedFile = this.transformFile(modulePath, {
const transformedCode = this.transformFile(modulePath, {
isInternalModule: false,
supportsDynamicImport: true,
supportsStaticESM: true,
});

const module = new SourceTextModule(transformedFile.code, {
const module = new SourceTextModule(transformedCode, {
context,
identifier: modulePath,
importModuleDynamically: this.linkModules.bind(this),
Expand Down Expand Up @@ -471,7 +471,7 @@ class Runtime {
const manualMock =
moduleName && this._resolver.getMockModule(from, moduleName);
if (
(!options || !options.isInternalModule) &&
!options?.isInternalModule &&
!isRequireActual &&
!moduleResource &&
manualMock &&
Expand All @@ -491,7 +491,9 @@ class Runtime {

let moduleRegistry;

if (!options || !options.isInternalModule) {
if (options?.isInternalModule) {
thymikee marked this conversation as resolved.
Show resolved Hide resolved
moduleRegistry = this._internalModuleRegistry;
} else {
if (
this._moduleRegistry.get(modulePath) ||
!this._isolatedModuleRegistry
Expand All @@ -500,8 +502,6 @@ class Runtime {
} else {
moduleRegistry = this._isolatedModuleRegistry;
}
} else {
moduleRegistry = this._internalModuleRegistry;
}

const module = moduleRegistry.get(modulePath);
Expand Down Expand Up @@ -641,7 +641,7 @@ class Runtime {
moduleRegistry: ModuleRegistry,
) {
if (path.extname(modulePath) === '.json') {
const text = stripBOM(fs.readFileSync(modulePath, 'utf8'));
const text = stripBOM(this.readFile(modulePath));

const transformedFile = this._scriptTransformer.transformJson(
modulePath,
Expand Down Expand Up @@ -965,7 +965,7 @@ class Runtime {
value: this._createRequireImplementation(localModule, options),
});

const transformedFile = this.transformFile(filename, options);
const transformedCode = this.transformFile(filename, options);

let compiledFunction: ModuleWrapper | null = null;

Expand All @@ -977,7 +977,7 @@ class Runtime {
if (typeof compileFunction === 'function') {
try {
compiledFunction = compileFunction(
transformedFile.code,
transformedCode,
this.constructInjectedModuleParameters(),
{
filename,
Expand All @@ -988,10 +988,7 @@ class Runtime {
throw handlePotentialSyntaxError(e);
}
} else {
const script = this.createScriptFromCode(
transformedFile.code,
filename,
);
const script = this.createScriptFromCode(transformedCode, filename);

const runScript = script.runInContext(
vmContext,
Expand All @@ -1005,7 +1002,7 @@ class Runtime {
}
}
} else {
const script = this.createScriptFromCode(transformedFile.code, filename);
const script = this.createScriptFromCode(transformedCode, filename);

const runScript = this._environment.runScript<RunScriptEvalResult>(
script,
Expand Down Expand Up @@ -1058,22 +1055,28 @@ class Runtime {
this._currentlyExecutingModulePath = lastExecutingModulePath;
}

private transformFile(filename: string, options?: InternalModuleOptions) {
private transformFile(
filename: string,
options?: InternalModuleOptions,
): string {
const source = this.readFile(filename);

if (options?.isInternalModule) {
return source;
}

const transformedFile = this._scriptTransformer.transform(
filename,
this._getFullTransformationOptions(options),
this._cacheFS[filename],
source,
);

// we only care about non-internal modules
if (!options || !options.isInternalModule) {
this._fileTransforms.set(filename, transformedFile);
}
this._fileTransforms.set(filename, transformedFile);

if (transformedFile.sourceMapPath) {
this._sourceMapRegistry[filename] = transformedFile.sourceMapPath;
}
return transformedFile;
return transformedFile.code;
}

private createScriptFromCode(scriptSource: string, filename: string) {
Expand Down Expand Up @@ -1303,7 +1306,7 @@ class Runtime {
resolve.paths = (moduleName: string) =>
this._requireResolvePaths(from.filename, moduleName);

const moduleRequire = (options && options.isInternalModule
const moduleRequire = (options?.isInternalModule
? (moduleName: string) =>
this.requireInternalModule(from.filename, moduleName)
: this.requireModuleOrMock.bind(
Expand Down Expand Up @@ -1635,6 +1638,18 @@ class Runtime {
xtest: this._environment.global.xtest,
};
}

private readFile(filename: Config.Path): string {
let source = this._cacheFS[filename];

if (!source) {
source = fs.readFileSync(filename, 'utf8');

this._cacheFS[filename] = source;
}

return source;
}
}

function invariant(condition: unknown, message?: string): asserts condition {
Expand Down