Skip to content

Commit

Permalink
Improve source map handling when instrumenting transformed code (#5739)
Browse files Browse the repository at this point in the history
  • Loading branch information
Matthew Preble committed Apr 14, 2020
1 parent 99c6fb1 commit c65d3c2
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 31 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -9,6 +9,7 @@

- `[expect]` Restore support for passing functions to `toHaveLength` matcher ([#9796](https://github.com/facebook/jest/pull/9796))
- `[jest-changed-files]` `--only-changed` should include staged files ([#9799](https://github.com/facebook/jest/pull/9799))
- `[jest-transform]` Improve source map handling when instrumenting transformed code ([#9811](https://github.com/facebook/jest/pull/9811))

### Chore & Maintenance

Expand Down
65 changes: 38 additions & 27 deletions packages/jest-transform/src/ScriptTransformer.ts
View file
Expand Up @@ -205,11 +205,15 @@ export default class ScriptTransformer {

private _instrumentFile(
filename: Config.Path,
content: string,
input: TransformedSource,
supportsDynamicImport: boolean,
supportsStaticESM: boolean,
): string {
const result = babelTransform(content, {
canMapToInput: boolean,
): TransformedSource {
const inputCode = typeof input === 'string' ? input : input.code;
const inputMap = typeof input === 'string' ? null : input.map;

const result = babelTransform(inputCode, {
auxiliaryCommentBefore: ' istanbul ignore next ',
babelrc: false,
caller: {
Expand All @@ -228,21 +232,19 @@ export default class ScriptTransformer {
cwd: this._config.rootDir,
exclude: [],
extension: false,
inputSourceMap: inputMap,
useInlineSourceMaps: false,
},
],
],
sourceMaps: canMapToInput ? 'both' : false,
});

if (result) {
const {code} = result;

if (code) {
return code;
}
if (result && result.code) {
return result as TransformResult;
}

return content;
return input;
}

private _getRealPath(filepath: Config.Path): Config.Path {
Expand Down Expand Up @@ -287,18 +289,14 @@ export default class ScriptTransformer {
const transformWillInstrument =
shouldCallTransform && transform && transform.canInstrument;

// If we handle the coverage instrumentation, we should try to map code
// coverage against original source with any provided source map
const mapCoverage = instrument && !transformWillInstrument;

if (code) {
// This is broken: we return the code, and a path for the source map
// directly from the cache. But, nothing ensures the source map actually
// matches that source code. They could have gotten out-of-sync in case
// two separate processes write concurrently to the same cache files.
return {
code,
mapCoverage,
mapCoverage: false,
originalCode: content,
sourceMapPath,
};
Expand Down Expand Up @@ -333,9 +331,8 @@ export default class ScriptTransformer {
//Could be a potential freeze here.
//See: https://github.com/facebook/jest/pull/5177#discussion_r158883570
const inlineSourceMap = sourcemapFromSource(transformed.code);

if (inlineSourceMap) {
transformed.map = inlineSourceMap.toJSON();
transformed.map = inlineSourceMap.toObject();
}
} catch (e) {
const transformPath = this._getTransformPath(filename);
Expand All @@ -347,22 +344,38 @@ 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) {
code = this._instrumentFile(
/**
* 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
* - we did not transform the source code
*
* Otherwise we cannot make any statements about how the instrumented code corresponds to the original code,
* and we should NOT emit any source maps
*
*/
const shouldEmitSourceMaps = (!!transform && !!map) || !transform;

const instrumented = this._instrumentFile(
filename,
transformed.code,
transformed,
supportsDynamicImport,
supportsStaticESM,
shouldEmitSourceMaps,
);

code =
typeof instrumented === 'string' ? instrumented : instrumented.code;
map = typeof instrumented === 'string' ? null : instrumented.map;
} else {
code = transformed.code;
}

if (transformed.map) {
if (map) {
const sourceMapContent =
typeof transformed.map === 'string'
? transformed.map
: JSON.stringify(transformed.map);
typeof map === 'string' ? map : JSON.stringify(map);
writeCacheFile(sourceMapPath, sourceMapContent);
} else {
sourceMapPath = null;
Expand All @@ -372,7 +385,7 @@ export default class ScriptTransformer {

return {
code,
mapCoverage,
mapCoverage: false,
originalCode: content,
sourceMapPath,
};
Expand All @@ -396,7 +409,6 @@ export default class ScriptTransformer {

let code = content;
let sourceMapPath: string | null = null;
let mapCoverage = false;

const willTransform =
!isInternalModule &&
Expand All @@ -415,12 +427,11 @@ export default class ScriptTransformer {

code = transformedSource.code;
sourceMapPath = transformedSource.sourceMapPath;
mapCoverage = transformedSource.mapCoverage;
}

return {
code,
mapCoverage,
mapCoverage: false,
originalCode: content,
sourceMapPath,
};
Expand Down
12 changes: 8 additions & 4 deletions packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap
View file
Expand Up @@ -80,7 +80,7 @@ exports[`ScriptTransformer transforms a file properly 1`] = `
/* istanbul ignore next */
function cov_25u22311x4() {
var path = "/fruits/banana.js";
var hash = "4be0f6184160be573fc43f7c2a5877c28b7ce249";
var hash = "3f8e915bed83285455a8a16aa04dc0cf5242d755";
var global = new Function("return this")();
var gcv = "__coverage__";
var coverageData = {
Expand All @@ -104,8 +104,9 @@ function cov_25u22311x4() {
},
f: {},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "4be0f6184160be573fc43f7c2a5877c28b7ce249"
hash: "3f8e915bed83285455a8a16aa04dc0cf5242d755"
};
var coverage = global[gcv] || (global[gcv] = {});
Expand All @@ -125,13 +126,14 @@ function cov_25u22311x4() {
cov_25u22311x4();
cov_25u22311x4().s[0]++;
module.exports = "banana";
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImJhbmFuYS5qcyJdLCJuYW1lcyI6WyJtb2R1bGUiLCJleHBvcnRzIl0sIm1hcHBpbmdzIjoiOzs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7QUFBQUEsTUFBTSxDQUFDQyxPQUFQLEdBQWlCLFFBQWpCIiwic291cmNlc0NvbnRlbnQiOlsibW9kdWxlLmV4cG9ydHMgPSBcImJhbmFuYVwiOyJdfQ==
`;

exports[`ScriptTransformer transforms a file properly 2`] = `
/* istanbul ignore next */
function cov_23yvu8etmu() {
var path = "/fruits/kiwi.js";
var hash = "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a";
var hash = "8b5afd38d79008f13ebc229b89ef82b12ee9447a";
var global = new Function("return this")();
var gcv = "__coverage__";
var coverageData = {
Expand Down Expand Up @@ -193,8 +195,9 @@ function cov_23yvu8etmu() {
"0": 0
},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a"
hash: "8b5afd38d79008f13ebc229b89ef82b12ee9447a"
};
var coverage = global[gcv] || (global[gcv] = {});
Expand All @@ -220,6 +223,7 @@ module.exports = () => {
cov_23yvu8etmu().s[1]++;
return "kiwi";
};
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImtpd2kuanMiXSwibmFtZXMiOlsibW9kdWxlIiwiZXhwb3J0cyJdLCJtYXBwaW5ncyI6Ijs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7O0FBQUFBLE1BQU0sQ0FBQ0MsT0FBUCxHQUFpQixNQUFNO0FBQUE7QUFBQTtBQUFBO0FBQUE7QUFBTSxDQUE3QiIsInNvdXJjZXNDb250ZW50IjpbIm1vZHVsZS5leHBvcnRzID0gKCkgPT4gXCJraXdpXCI7Il19
`;

exports[`ScriptTransformer uses multiple preprocessors 1`] = `
Expand Down
84 changes: 84 additions & 0 deletions packages/jest-transform/src/__tests__/script_transformer.test.js
View file
Expand Up @@ -535,6 +535,90 @@ describe('ScriptTransformer', () => {
expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
});

it('should write a source map for the instrumented file when transformed', () => {
const transformerConfig = {
...config,
transform: [['^.+\\.js$', 'preprocessor-with-sourcemaps']],
};
const scriptTransformer = new ScriptTransformer(transformerConfig);

const map = {
mappings: ';AAAA',
version: 3,
};

// A map from the original source to the instrumented output
/* eslint-disable sort-keys */
const instrumentedCodeMap = {
version: 3,
sources: ['banana.js'],
names: ['content'],
mappings: ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,OAAO',
sourcesContent: ['content'],
};
/* eslint-enable */

require('preprocessor-with-sourcemaps').process.mockReturnValue({
code: 'content',
map,
});

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
result.sourceMapPath,
JSON.stringify(instrumentedCodeMap),
expect.anything(),
);

// Inline source map allows debugging of original source when running instrumented code
expect(result.code).toContain('//# sourceMappingURL');
});

it('should write a source map for the instrumented file when not transformed', () => {
const scriptTransformer = new ScriptTransformer(config);

// A map from the original source to the instrumented output
/* eslint-disable sort-keys */
const instrumentedCodeMap = {
version: 3,
sources: ['banana.js'],
names: ['module', 'exports'],
mappings:
';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,MAAM,CAACC,OAAP,GAAiB,QAAjB',
sourcesContent: ['module.exports = "banana";'],
};
/* eslint-enable */

require('preprocessor-with-sourcemaps').process.mockReturnValue({
code: 'content',
map: null,
});

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
expect(writeFileAtomic.sync).toBeCalledWith(
result.sourceMapPath,
JSON.stringify(instrumentedCodeMap),
expect.anything(),
);

// Inline source map allows debugging of original source when running instrumented code
expect(result.code).toContain('//# sourceMappingURL');
});

it('passes expected transform options to getCacheKey', () => {
config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]};
const scriptTransformer = new ScriptTransformer(config);
Expand Down

0 comments on commit c65d3c2

Please sign in to comment.