Skip to content

Commit

Permalink
chore(transform): refactor API to pass an options bag around rather t…
Browse files Browse the repository at this point in the history
…han multiple boolean options
  • Loading branch information
SimenB committed Oct 31, 2020
1 parent 95169d3 commit 4ece96b
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 82 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- `[eslint-config-fb-strict]` Move package from this repo to `fbjs` repo ([#10739](https://github.com/facebook/jest/pull/10739))
- `[examples]` Update TypeScript example to show use of newer Jest types ([#10399](https://github.com/facebook/jest/pull/10399))
- `[jest-cli]` chore: standardize files and folder names ([#10698](https://github.com/facebook/jest/pull/10698))
- `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753))

### Performance

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-reporters/src/generateEmptyCoverage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default function (
const {code} = new ScriptTransformer(config).transformSource(
filename,
source,
true,
{instrument: true},
);
// TODO: consider passing AST
const extracted = readInitialCoverage(code);
Expand Down
108 changes: 44 additions & 64 deletions packages/jest-transform/src/ScriptTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {sync as writeFileAtomic} from 'write-file-atomic';
import {addHook} from 'pirates';
import type {
Options,
TransformOptions,
TransformResult,
TransformedSource,
Transformer,
Expand Down Expand Up @@ -62,6 +63,14 @@ async function waitForPromiseWithCleanup(
}
}

// https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49267
declare module '@babel/core' {
interface TransformCaller {
supportsExportNamespaceFrom?: boolean;
supportsTopLevelAwait?: boolean;
}
}

export default class ScriptTransformer {
private _cache: ProjectCache;
private _config: Config.ProjectConfig;
Expand Down Expand Up @@ -93,9 +102,7 @@ export default class ScriptTransformer {
private _getCacheKey(
fileData: string,
filename: Config.Path,
instrument: boolean,
supportsDynamicImport: boolean,
supportsStaticESM: boolean,
options: TransformOptions,
): string {
const configString = this._cache.configString;
const transformer = this._getTransformer(filename);
Expand All @@ -105,10 +112,12 @@ export default class ScriptTransformer {
.update(
transformer.getCacheKey(fileData, filename, configString, {
config: this._config,
instrument,
instrument: options.instrument,
rootDir: this._config.rootDir,
supportsDynamicImport,
supportsStaticESM,
supportsDynamicImport: options.supportsDynamicImport,
supportsExportNamespaceFrom: options.supportsExportNamespaceFrom,
supportsStaticESM: options.supportsStaticESM,
supportsTopLevelAwait: options.supportsTopLevelAwait,
}),
)
.update(CACHE_VERSION)
Expand All @@ -117,7 +126,7 @@ export default class ScriptTransformer {
return createHash('md5')
.update(fileData)
.update(configString)
.update(instrument ? 'instrument' : '')
.update(options.instrument ? 'instrument' : '')
.update(filename)
.update(CACHE_VERSION)
.digest('hex');
Expand All @@ -127,22 +136,14 @@ export default class ScriptTransformer {
private _getFileCachePath(
filename: Config.Path,
content: string,
instrument: boolean,
supportsDynamicImport: boolean,
supportsStaticESM: boolean,
options: TransformOptions,
): Config.Path {
const baseCacheDir = HasteMap.getCacheFilePath(
this._config.cacheDirectory,
'jest-transform-cache-' + this._config.name,
VERSION,
);
const cacheKey = this._getCacheKey(
content,
filename,
instrument,
supportsDynamicImport,
supportsStaticESM,
);
const cacheKey = this._getCacheKey(content, filename, options);
// Create sub folders based on the cacheKey to avoid creating one
// directory with many files.
const cacheDir = path.join(baseCacheDir, cacheKey[0] + cacheKey[1]);
Expand Down Expand Up @@ -213,9 +214,8 @@ export default class ScriptTransformer {
private _instrumentFile(
filename: Config.Path,
input: TransformedSource,
supportsDynamicImport: boolean,
supportsStaticESM: boolean,
canMapToInput: boolean,
options: TransformOptions,
): TransformedSource {
const inputCode = typeof input === 'string' ? input : input.code;
const inputMap = typeof input === 'string' ? null : input.map;
Expand All @@ -225,8 +225,10 @@ export default class ScriptTransformer {
babelrc: false,
caller: {
name: '@jest/transform',
supportsDynamicImport,
supportsStaticESM,
supportsDynamicImport: options.supportsDynamicImport,
supportsExportNamespaceFrom: options.supportsExportNamespaceFrom,
supportsStaticESM: options.supportsStaticESM,
supportsTopLevelAwait: options.supportsTopLevelAwait,
},
configFile: false,
filename,
Expand Down Expand Up @@ -260,23 +262,14 @@ export default class ScriptTransformer {
this._getTransformer(filepath);
}

// TODO: replace third argument with TransformOptions in Jest 26
transformSource(
filepath: Config.Path,
content: string,
instrument: boolean,
supportsDynamicImport = false,
supportsStaticESM = false,
options: TransformOptions,
): TransformResult {
const filename = tryRealpath(filepath);
const transform = this._getTransformer(filename);
const cacheFilePath = this._getFileCachePath(
filename,
content,
instrument,
supportsDynamicImport,
supportsStaticESM,
);
const cacheFilePath = this._getFileCachePath(filename, content, options);
let sourceMapPath: Config.Path | null = cacheFilePath + '.map';
// Ignore cache if `config.cache` is set (--no-cache)
let code = this._config.cache ? readCodeCacheFile(cacheFilePath) : null;
Expand Down Expand Up @@ -306,11 +299,12 @@ export default class ScriptTransformer {
};

if (transform && shouldCallTransform) {
const processed = transform.process(content, filename, this._config, {
instrument,
supportsDynamicImport,
supportsStaticESM,
});
const processed = transform.process(
content,
filename,
this._config,
options,
);

if (typeof processed === 'string') {
transformed.code = processed;
Expand Down Expand Up @@ -344,7 +338,7 @@ export default class ScriptTransformer {

// Apply instrumentation to the code if necessary, keeping the instrumented code and new map
let map = transformed.map;
if (!transformWillInstrument && instrument) {
if (!transformWillInstrument && options.instrument) {
/**
* We can map the original source code to the instrumented code ONLY if
* - the process of transforming the code produced a source map e.g. ts-jest
Expand All @@ -360,9 +354,8 @@ export default class ScriptTransformer {
const instrumented = this._instrumentFile(
filename,
transformed,
supportsDynamicImport,
supportsStaticESM,
shouldEmitSourceMaps,
options,
);

code =
Expand Down Expand Up @@ -392,15 +385,10 @@ export default class ScriptTransformer {
private _transformAndBuildScript(
filename: Config.Path,
options: Options,
instrument: boolean,
transformOptions: TransformOptions,
fileSource?: string,
): TransformResult {
const {
isCoreModule,
isInternalModule,
supportsDynamicImport,
supportsStaticESM,
} = options;
const {isCoreModule, isInternalModule} = options;
const content = stripShebang(
fileSource || fs.readFileSync(filename, 'utf8'),
);
Expand All @@ -411,16 +399,14 @@ export default class ScriptTransformer {
const willTransform =
!isInternalModule &&
!isCoreModule &&
(this.shouldTransform(filename) || instrument);
(this.shouldTransform(filename) || transformOptions.instrument);

try {
if (willTransform) {
const transformedSource = this.transformSource(
filename,
content,
instrument,
supportsDynamicImport,
supportsStaticESM,
transformOptions,
);

code = transformedSource.code;
Expand Down Expand Up @@ -459,7 +445,7 @@ export default class ScriptTransformer {
const result = this._transformAndBuildScript(
filename,
options,
instrument,
{...options, instrument},
fileSource,
);

Expand All @@ -475,22 +461,15 @@ export default class ScriptTransformer {
options: Options,
fileSource: string,
): string {
const {
isCoreModule,
isInternalModule,
supportsDynamicImport,
supportsStaticESM,
} = options;
const {isCoreModule, isInternalModule} = options;
const willTransform =
!isInternalModule && !isCoreModule && this.shouldTransform(filename);

if (willTransform) {
const {code: transformedJsonSource} = this.transformSource(
filename,
fileSource,
false,
supportsDynamicImport,
supportsStaticESM,
{...options, instrument: false},
);
return transformedJsonSource;
}
Expand All @@ -501,14 +480,17 @@ export default class ScriptTransformer {
requireAndTranspileModule<ModuleType = unknown>(
moduleName: string,
callback?: (module: ModuleType) => void,
transformOptions?: TransformOptions,
): ModuleType;
requireAndTranspileModule<ModuleType = unknown>(
moduleName: string,
callback?: (module: ModuleType) => Promise<void>,
transformOptions?: TransformOptions,
): Promise<ModuleType>;
requireAndTranspileModule<ModuleType = unknown>(
moduleName: string,
callback?: (module: ModuleType) => void | Promise<void>,
transformOptions: TransformOptions = {instrument: false},
): ModuleType | Promise<ModuleType> {
// Load the transformer to avoid a cycle where we need to load a
// transformer in order to transform it in the require hooks
Expand All @@ -520,9 +502,7 @@ export default class ScriptTransformer {
try {
transforming = true;
return (
// we might wanna do `supportsDynamicImport` at some point
this.transformSource(filename, code, false, false, false).code ||
code
this.transformSource(filename, code, transformOptions).code || code
);
} finally {
transforming = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,10 @@ Object {
},
"instrument": true,
"rootDir": "/",
"supportsDynamicImport": false,
"supportsStaticESM": false,
"supportsDynamicImport": undefined,
"supportsExportNamespaceFrom": undefined,
"supportsStaticESM": undefined,
"supportsTopLevelAwait": undefined,
}
`;

Expand Down
20 changes: 5 additions & 15 deletions packages/jest-transform/src/__tests__/script_transformer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,9 +473,7 @@ describe('ScriptTransformer', () => {

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig({collectCoverage: true}),
);
expect(result.sourceMapPath).toBeNull();
expect(writeFileAtomic.sync).toBeCalledTimes(1);
Expand Down Expand Up @@ -532,9 +530,7 @@ describe('ScriptTransformer', () => {

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig({collectCoverage: true}),
);
expect(result.sourceMapPath).toBeFalsy();
expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -571,9 +567,7 @@ describe('ScriptTransformer', () => {

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig({collectCoverage: true}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
Expand Down Expand Up @@ -609,9 +603,7 @@ describe('ScriptTransformer', () => {

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig({collectCoverage: true}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
Expand All @@ -631,9 +623,7 @@ describe('ScriptTransformer', () => {

scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig({collectCoverage: true}),
);

const {getCacheKey} = require('test_preprocessor');
Expand Down

0 comments on commit 4ece96b

Please sign in to comment.