sucrase
sucrase copied to clipboard
Injecting code/comments after transformation - c8 coverage fails
Thanks for all your work on sucrase. It's an amazing project.
I'm a big fan of c8 for code coverage checking, and it works almost perfectly with sucrase.
Unfortunately, when you are importing other modules/files, sucrase introduces some branching logic, when it inserts this line:
"use strict";const _jsxFileName = "src/index.tsx"; function _interopRequireDefault(obj) { return obj && obj.__esModule ? obj : { default: obj }; }var _react = require('react'); var _react2 = _interopRequireDefault(_react);
Note the
obj.__esModule ? obj : { default: obj }
is the specific part that fails c8
This results in my code coverage claiming line 1
has untested branching logic.
If I put the following hack in, it works perfectly:
This is no longer working, see this comment below
diff --git a/src/index.ts b/src/index.ts
index feb4362..1e2329b 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -54,6 +54,9 @@ export function transform(code: string, options: Options): TransformResult {
sourceMap: computeSourceMap(result.code, options.filePath, options.sourceMapOptions),
};
}
+
+ result.code = `/* c8 ignore next */\n${result.code}`;
+
return result;
} catch (e) {
if (options.filePath) {
But obviously this isn't a great solution. Do you have any recommendations on how we could:
- Remove that branch logic or the
_interopRequireDefault
entirely - Or, allowing adding custom prepends to transformation results. e.g.:
require("sucrase/dist/register").registerAll({
prependCode: `/* c8 ignore next */`
})
Example
My ReactFast project shows the problem if you run npm run test
. You'll see a coverage report claiming line 1 is uncovered.
Hey @markwylde , thanks for the report. I think ideally Sucrase would integrate well with other tools by default, so I'd certainly be open to including /* c8 ignore next */
comments by default (except with production: true
) if they make c8 coverage more accurate.
I tried playing around with it myself (thanks for sharing an example repo!) and had some trouble narrowing it down. I tried changing the output code like you did, and I also tried changes to https://github.com/alangpierce/sucrase/blob/main/src/HelperManager.ts where the helpers are defined. A few observations:
- I couldn't figure out a place to put
/* c8 ignore next */
or/* c8 ignore start */
in theinteropRequireDefault
helper code to avoid the uncovered line, but maybe there's a way to make it work. - Adding just a newline (no
/* c8 ignore next */
) to the start of the file seemed to make the problem go away. - Removing the snippet
.replace(/\s+/g, " ")
(which removes newlines and spaces for helpers, making them one long line) seemed to make the problem go away. - Editing
App.tsx
to have a blank line at the top seemed to make the problem go away.
I'm a bit confused by those last three behaviors since it looks like AST-equivalent files are giving different results. I'm now wondering if this is a c8
bug or a situation where c8
decides to give up on lines that are too long, or something like that.
In any case, one concern with adding a newline at the start of the code is that Sucrase guarantees that line numbers are the same before and after transpile (e.g. so stack traces are accurate even without source maps). Adding extra newlines to the top of the file would break that guarantee.
If you wanted to track down the problem a bit more and figure out a concrete code change, I'd probably be happy to accept a PR that makes Sucrase more c8-friendly. I probably won't have time to dig much into this issue much myself, though.
As a separate note, if you're trying to work around this in a more immediate timeframe, I think your best option is to write your own version of the addHook
function from https://github.com/alangpierce/sucrase/blob/main/src/register.ts and then include that file with -r
. (Your version will probably need to be written in plain JS.) It's just ~15 lines, and you can modify transformedCode
there if you want without needing any changes in Sucrase.
I ran into exactly the same issue with
transform: { "\\.tsx?$": "@sucrase/jest-plugin" },
in my Jest config and the default coverage provider being "babel".
But I cannot tell why it works in one project configured the same way and fails in the other. Both of these projects have 100% coverage threshold for everything. Both projects use
"@sucrase/jest-plugin": "^2.2.0",
Thanks @alangpierce for your detailed response. I really appreciate it and sorry I haven't responded.
I keep coming back to this, hacking away at different solutions, but can't really come up with something that feels right.
For some reason my previous solution, above, doesn't work anymore. Might be a bug with c8 not handling ignores.
Instead, I'm now doing:
diff --git a/src/HelperManager.ts b/src/HelperManager.ts
index a4879c9..7e24e18 100644
--- a/src/HelperManager.ts
+++ b/src/HelperManager.ts
@@ -23,6 +23,8 @@ const HELPERS: {[name: string]: string} = {
function interopRequireDefault(obj) {
return obj && obj.__esModule ? obj : { default: obj };
}
+ _interopRequireDefault({});
+ _interopRequireDefault({ __esModule: {} });
`,
createNamedExportFrom: `
function createNamedExportFrom(obj, localName, importedName) {
This fixes the issue, and confirms my initial thought that c8 is reporting correctly, the _interopRequireDefault
function is basically not getting covered.
Again, it's not a pretty solution, and I'm still struggling to come up with better ways.
@alangpierce have you had any ideas/thoughts about the "HelperManager" and whether eventually those could be deprecated and instead actually transform the code to return a predictable object? It sounds like an incredibly hard thing to do, and at this stage I'm not even sure if possible.
I'm running into this too with the default coverage tooling for Jest (istanbul). My workaround so far is to ignore anything that looks like a helper in the first line of the transpiled source:
/**
* Inserts comments to ignore inserted helper functions.
*/
function ignoringHelperFunctions(code: string): string {
const firstLineEnding = code.indexOf('\n');
const firstLine = code.slice(0, firstLineEnding);
const rest = code.slice(firstLineEnding);
return (
firstLine.replace(/\bfunction _/g, '/* istanbul ignore next */function _') +
rest
);
}
function process(src: string, filename: string) {
…
const codeIgnoringHelperFunctions = ignoringHelperFunctions(code);
return {
code: `${codeIgnoringHelperFunctions}\n${suffix}`,
map: sourceMap,
};
}
It seems like we could cover (no pun intended) most of the coverage issues with this plugin just by doing something like the above. Could even throw in the c8
comment for good measure.
I like the idea of just adding those comments here, but doesn't this require to update the source maps, too? 🤔 I guess, it requires some investigation 😅
This is how babel-jest
handles it: https://github.com/facebook/jest/blob/3a8696bcf0e02a5f59080123d1a20e8b6d66d8df/packages/babel-jest/src/index.ts#L52
I tested it again with
-
"@sucrase/jest-plugin": "^3.0.0"
-
"sucrase": "^3.28.0"
The issue doesn't appear for me anymore 🙃