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
chore(transform): refactor API to pass an options bag around rather than multiple boolean options #10753
chore(transform): refactor API to pass an options bag around rather than multiple boolean options #10753
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,7 @@ import handlePotentialSyntaxError from './enhanceUnexpectedTokenMessage'; | |
import shouldInstrument from './shouldInstrument'; | ||
import type { | ||
Options, | ||
TransformOptions, | ||
TransformResult, | ||
TransformedSource, | ||
Transformer, | ||
|
@@ -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); | ||
|
@@ -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) | ||
|
@@ -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'); | ||
|
@@ -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]); | ||
|
@@ -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; | ||
|
@@ -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, | ||
|
@@ -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; | ||
|
@@ -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; | ||
|
@@ -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 | ||
|
@@ -359,9 +345,8 @@ export default class ScriptTransformer { | |
const instrumented = this._instrumentFile( | ||
filename, | ||
transformed, | ||
supportsDynamicImport, | ||
supportsStaticESM, | ||
shouldEmitSourceMaps, | ||
options, | ||
); | ||
|
||
code = | ||
|
@@ -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'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ugh, too painful to fix the tests, can circle back to it later. I pushed WIP stuff here: SimenB@31d67e2 |
||
); | ||
|
@@ -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; | ||
|
@@ -458,7 +436,7 @@ export default class ScriptTransformer { | |
const result = this._transformAndBuildScript( | ||
filename, | ||
options, | ||
instrument, | ||
{...options, instrument}, | ||
fileSource, | ||
); | ||
|
||
|
@@ -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; | ||
} | ||
|
@@ -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 | ||
|
@@ -519,9 +493,7 @@ export default class ScriptTransformer { | |
try { | ||
transforming = true; | ||
return ( | ||
// we might wanna do `supportsDynamicImport` at some point | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. allowing consumers of this API to decide seems best |
||
this.transformSource(filename, code, false, false, false).code || | ||
code | ||
this.transformSource(filename, code, transformOptions).code || code | ||
); | ||
} finally { | ||
transforming = false; | ||
|
@@ -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; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😅