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

Produce source maps when instrumenting code #9460

Merged
merged 3 commits into from Jan 25, 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 @@ -5,6 +5,7 @@
### Fixes

- `[jest-snapshot]` Downgrade semver to v6 to support node 8 ([#9451](https://github.com/facebook/jest/pull/9451))
- `[jest-transform]` Correct sourcemap behavior for transformed and instrumented code ([#9460](https://github.com/facebook/jest/pull/9460))

### Chore & Maintenance

Expand Down
57 changes: 43 additions & 14 deletions packages/jest-transform/src/ScriptTransformer.ts
Expand Up @@ -191,8 +191,12 @@ export default class ScriptTransformer {
return transform;
}

private _instrumentFile(filename: Config.Path, content: string): string {
const result = babelTransform(content, {
private _instrumentFile(
filename: Config.Path,
input: TransformedSource,
canMapToInput: boolean,
): TransformedSource {
const result = babelTransform(input.code, {
auxiliaryCommentBefore: ' istanbul ignore next ',
babelrc: false,
caller: {
Expand All @@ -209,21 +213,27 @@ export default class ScriptTransformer {
// files outside `cwd` will not be instrumented
cwd: this._config.rootDir,
exclude: [],
// Needed for correct coverage as soon as we start storing a source map of the instrumented code
inputSourceMap: input.map,
useInlineSourceMaps: false,
},
],
],
/**
* It's necessary to be able to map back to original source from the instrumented code.
* The inline map is needed for debugging functionality, and exposing it as a separate file is needed
* for mapping stack traces. It's convenient to use 'both' here and avoid extracting the source map.
*
* Previous behavior of emitting no map when we can't map back to original source is preserved.
*/
sourceMaps: canMapToInput ? 'both' : false,
});

if (result) {
const {code} = result;
mbpreble marked this conversation as resolved.
Show resolved Hide resolved

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could create a type guard function and avoid the cast, but it's not any more type safe. so meh

}

return content;
return {code: input.code};
}

private _getRealPath(filepath: Config.Path): Config.Path {
Expand Down Expand Up @@ -312,17 +322,36 @@ 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(filename, transformed.code);
/**
* 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,
shouldEmitSourceMaps,
);
code = instrumented.code;

if (instrumented.map) {
map = 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 Down
Expand Up @@ -78,7 +78,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 @@ -102,8 +102,9 @@ function cov_25u22311x4() {
},
f: {},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "4be0f6184160be573fc43f7c2a5877c28b7ce249"
hash: "3f8e915bed83285455a8a16aa04dc0cf5242d755"
};
var coverage = global[gcv] || (global[gcv] = {});

Expand All @@ -122,13 +123,14 @@ function cov_25u22311x4() {

cov_25u22311x4().s[0]++;
module.exports = "banana";
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImJhbmFuYS5qcyJdLCJuYW1lcyI6WyJtb2R1bGUiLCJleHBvcnRzIl0sIm1hcHBpbmdzIjoiOzs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7OztBQUFBQSxNQUFNLENBQUNDLE9BQVAsR0FBaUIsUUFBakIiLCJzb3VyY2VzQ29udGVudCI6WyJtb2R1bGUuZXhwb3J0cyA9IFwiYmFuYW5hXCI7Il19
`;

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 @@ -190,8 +192,9 @@ function cov_23yvu8etmu() {
"0": 0
},
b: {},
inputSourceMap: null,
_coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9",
hash: "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a"
hash: "8b5afd38d79008f13ebc229b89ef82b12ee9447a"
};
var coverage = global[gcv] || (global[gcv] = {});

Expand All @@ -216,6 +219,7 @@ module.exports = () => {
cov_23yvu8etmu().s[1]++;
return "kiwi";
};
//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImtpd2kuanMiXSwibmFtZXMiOlsibW9kdWxlIiwiZXhwb3J0cyJdLCJtYXBwaW5ncyI6Ijs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7QUFBQUEsTUFBTSxDQUFDQyxPQUFQLEdBQWlCLE1BQU07QUFBQTtBQUFBO0FBQUE7QUFBQTtBQUFNLENBQTdCIiwic291cmNlc0NvbnRlbnQiOlsibW9kdWxlLmV4cG9ydHMgPSAoKSA9PiBcImtpd2lcIjsiXX0=
`;

exports[`ScriptTransformer uses multiple preprocessors 1`] = `
Expand Down
100 changes: 91 additions & 9 deletions packages/jest-transform/src/__tests__/script_transformer.test.js
Expand Up @@ -400,9 +400,7 @@ describe('ScriptTransformer', () => {

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig(),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
const mapStr = JSON.stringify(map);
Expand Down Expand Up @@ -433,9 +431,7 @@ describe('ScriptTransformer', () => {

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

const result = scriptTransformer.transform(
'/fruits/banana.js',
makeGlobalConfig({
collectCoverage: true,
}),
makeGlobalConfig(),
);
expect(result.sourceMapPath).toEqual(expect.any(String));
expect(writeFileAtomic.sync).toBeCalledTimes(2);
Expand Down Expand Up @@ -541,6 +535,94 @@ describe('ScriptTransformer', () => {
expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
});

it('should write a source map for the instrumented file when transformed', () => {
const transformerConfig = {
...config,
mbpreble marked this conversation as resolved.
Show resolved Hide resolved
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),
{
encoding: 'utf8',
},
);

// 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),
{
encoding: 'utf8',
},
);

// 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