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 Nov 4, 2020
1 parent 30b6cee commit 59c2653
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 78 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -7,6 +7,7 @@
### Fixes

- `[jest-transform]` Show enhanced `SyntaxError` message for all `SyntaxError`s ([#10749](https://github.com/facebook/jest/pull/10749))
- `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753))

### Chore & Maintenance

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-reporters/src/generateEmptyCoverage.ts
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: 36 additions & 72 deletions packages/jest-transform/src/ScriptTransformer.ts
Expand Up @@ -28,6 +28,7 @@ import handlePotentialSyntaxError from './enhanceUnexpectedTokenMessage';
import shouldInstrument from './shouldInstrument';
import type {
Options,
TransformOptions,
TransformResult,
TransformedSource,
Transformer,
Expand Down Expand Up @@ -92,9 +93,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 @@ -104,10 +103,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 @@ -116,7 +117,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 @@ -126,22 +127,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 @@ -212,9 +205,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 @@ -224,8 +216,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 @@ -259,23 +253,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 @@ -305,11 +290,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 @@ -343,7 +329,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 @@ -359,9 +345,8 @@ export default class ScriptTransformer {
const instrumented = this._instrumentFile(
filename,
transformed,
supportsDynamicImport,
supportsStaticESM,
shouldEmitSourceMaps,
options,
);

code =
Expand Down Expand Up @@ -391,15 +376,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 @@ -410,16 +390,14 @@ export default class ScriptTransformer {
const willTransform =
!isInternalModule &&
!isCoreModule &&
(this.shouldTransform(filename) || instrument);
(transformOptions.instrument || this.shouldTransform(filename));

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

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

Expand All @@ -474,22 +452,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 @@ -500,14 +471,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 @@ -519,9 +493,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 Expand Up @@ -562,14 +534,6 @@ export default class ScriptTransformer {
return module;
}

/**
* @deprecated use `this.shouldTransform` instead
*/
// @ts-expect-error: Unused and private - remove in Jest 25
private _shouldTransform(filename: Config.Path): boolean {
return this.shouldTransform(filename);
}

shouldTransform(filename: Config.Path): boolean {
const ignoreRegexp = this._cache.ignorePatternsRegExp;
const isIgnored = ignoreRegexp ? ignoreRegexp.test(filename) : false;
Expand Down
Expand Up @@ -71,8 +71,10 @@ Object {
},
"instrument": true,
"rootDir": "/",
"supportsDynamicImport": false,
"supportsStaticESM": false,
"supportsDynamicImport": undefined,
"supportsExportNamespaceFrom": undefined,
"supportsStaticESM": undefined,
"supportsTopLevelAwait": undefined,
}
`;

Expand Down
Expand Up @@ -338,7 +338,7 @@ describe('ScriptTransformer', () => {
};
const scriptTransformer = new ScriptTransformer(config);
expect(() =>
scriptTransformer.transformSource('sample.js', '', false),
scriptTransformer.transformSource('sample.js', '', {instrument: false}),
).toThrow('Jest: a transform must export a `process` function.');
});

Expand All @@ -355,7 +355,7 @@ describe('ScriptTransformer', () => {
};
const scriptTransformer = new ScriptTransformer(config);
expect(() =>
scriptTransformer.transformSource('sample.js', '', false),
scriptTransformer.transformSource('sample.js', '', {instrument: false}),
).toThrow('Jest: a transform must export a `process` function.');
});

Expand All @@ -366,7 +366,7 @@ describe('ScriptTransformer', () => {
};
const scriptTransformer = new ScriptTransformer(config);
expect(() =>
scriptTransformer.transformSource('sample.js', '', false),
scriptTransformer.transformSource('sample.js', '', {instrument: false}),
).not.toThrow();
});

Expand Down

0 comments on commit 59c2653

Please sign in to comment.