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 2 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
58 changes: 44 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,28 @@ 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,
},
],
],
/**
* Necessary to be able to map back to original source from the instrumented code
mbpreble marked this conversation as resolved.
Show resolved Hide resolved
* the inline map is needed for debugging functionality
* and exposing it as a separate file is needed for e.g. mapping stack traces
* convenient to use 'both' here and avoid extracting the source map
*
* Prior 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 {code: result.code, map: result.map};
mbpreble marked this conversation as resolved.
Show resolved Hide resolved
}

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

private _getRealPath(filepath: Config.Path): Config.Path {
Expand Down Expand Up @@ -312,17 +323,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 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', () => {
config = {
...config,
mbpreble marked this conversation as resolved.
Show resolved Hide resolved
transform: [['^.+\\.js$', 'preprocessor-with-sourcemaps']],
};
const scriptTransformer = new ScriptTransformer(config);

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 sort-keys */
mbpreble marked this conversation as resolved.
Show resolved Hide resolved

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 sort-keys */
mbpreble marked this conversation as resolved.
Show resolved Hide resolved

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