Xunnamius / babel-plugin-explicit-exports-references

⚡ Transforms all internal references to a module's exports such that each reference starts with "module.exports" instead of directly referencing an internal name.
https://npm.im/babel-plugin-explicit-exports-references
MIT License
19 stars 3 forks source link

Transforming assignment expressions is unstable when collecting coverage (default reporter failing) #2

Open daetal-us opened 3 years ago

daetal-us commented 3 years ago

Added the plugin to my Babel config. Works great running tests. However running with coverage (with default reporter) everything kind of blows up. Lots of reports of “Container” being falsy attributed to this plugin. Still trying to identify root cause.

Xunnamius commented 3 years ago

You can enable debugging to get a readout on what this plugin is transforming and how with DEBUG='babel-plugin-explicit-exports-references:*' npx jest your-test-file-name.

And maybe I can help by giving a rundown on how this plugin transforms ASTs:

  1. It begins by looking for default and named export declarations

  2. For default exports, it looks for function declarations and class declarations that have ids (identifiers, i.e. variable names), like export default function main() {}, and updates any references to that id.

  3. For named exports, it looks for function and class declarations too, but also variable declarations like export const foo = 5; and export { x as default, y, x as z };. Enums are explicitly ignored. There might be more things that need to be ignored that are being mistakenly transformed (good place to check for an issue). Any references to each id/specifier this plugin comes across are updated.

  4. When updating references, only two kinds of are considered: identifiers and assignment expressions. All other refs are ignored, including TypeScript types and identifiers within their own declarations. If there's a problem happening, assignment expressions are another good place to check. Replacing them might be a tad aggressive, but I wasn't able to concoct any failing syntax. If you comment out these lines and things start working, then the problem is assignment expression replacement.

Once I put the final touches on another project I'm working on, I'll flesh out the documentation for this plugin with this and other important info 😅

Xunnamius commented 3 years ago

@daetal-us And here's an example of the plugin's debug output when it's functioning properly using the default reporter (and --coverage):

Expand

``` DEBUG='babel-plugin-explicit-exports-references:*' npx jest unit-utils --coverage babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index encountered specifier "ComponentAction as ComponentAction" +1ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 3 references to named export "ComponentAction" +0ms babel-plugin-explicit-exports-references:index (an export specifier reference was skipped) +0ms babel-plugin-explicit-exports-references:index (an TypeScript type reference was skipped) +1ms babel-plugin-explicit-exports-references:index encountered named export +1ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "PRIVILEGED_DEPS_URI" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +1ms babel-plugin-explicit-exports-references:index updating 1 references to named export "GLOBAL_PIPELINE_CONFIG_URI " +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "UPLOADED_METADATA_PATH" +0 ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "GIT_MIN_VERSION" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "invokeComponentAction" +0m s babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +103ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "ComponentActionError" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +26ms babel-plugin-explicit-exports-references:index (ignored) +0ms babel-plugin-explicit-exports-references:index encountered named export +59ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "setupEnv" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +1ms babel-plugin-explicit-exports-references:index encountered named export +45ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "uploadPaths" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +1ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "downloadPaths" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "cachePaths" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "uncachePaths" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "cloneRepository" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +76ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "installDependencies" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +1ms babel-plugin-explicit-exports-references:index updating 3 references to named export "installPeerDeps" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +2ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "installPrivilegedDependenc ies" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 1 references to named export "installNode" +1ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +76ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 2 references to named export "DEFAULT_MAX_RETRIES" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "SequenceExpression" was skipped) +0m s babel-plugin-explicit-exports-references:index encountered named export +1ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 2 references to named export "DEFAULT_MIN_DELAY" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index encountered specifier "retry as attempt" +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 3 references to named export "retry" (exported as "attem pt") +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index (an export specifier reference was skipped) +0ms babel-plugin-explicit-exports-references:index (an TypeScript type reference was skipped) +0ms babel-plugin-explicit-exports-references:index encountered named export +0ms babel-plugin-explicit-exports-references:index mode: named +0ms babel-plugin-explicit-exports-references:index updating 3 references to named export "retry" +0ms babel-plugin-explicit-exports-references:index (unsupported reference type "ExportNamedDeclaration" was skipped) +0ms babel-plugin-explicit-exports-references:index (an export specifier reference was skipped) +0ms babel-plugin-explicit-exports-references:index (an TypeScript type reference was skipped) +0ms PASS test/unit-utils.test.ts -------------|---------|----------|---------|---------|------------------- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s -------------|---------|----------|---------|---------|------------------- All files | 92.45 | 90.12 | 95.24 | 92.26 | src | 45.45 | 0 | 66.67 | 45.45 | error.ts | 100 | 100 | 100 | 100 | index.ts | 36.84 | 0 | 50 | 36.84 | 38-58 src/utils | 100 | 98.65 | 100 | 100 | env.ts | 100 | 100 | 100 | 100 | github.ts | 100 | 100 | 100 | 100 | install.ts | 100 | 100 | 100 | 100 | retry.ts | 100 | 96.55 | 100 | 100 | 39 types | 0 | 0 | 0 | 0 | global.ts | 0 | 0 | 0 | 0 | -------------|---------|----------|---------|---------|------------------- Test Suites: 1 passed, 1 total Tests: 47 passed, 47 total Snapshots: 0 total Time: 1.125 s Ran all test suites matching /unit-utils/i. ```

installPrivilegedDependencies and installDependencies in the logs above both refer to installPeerDeps internally, and the debug shows all references to these exported functions being transformed

daetal-us commented 3 years ago

Super helpful. It does not appear to be due to assignment expression. I've narrowed down on case to an import from another file - its just an exported string const. For some reason the traversal context is different when the coverage param is enabled - the parent node is different and the container property of the node is empty (in coverage context)

e.g.

// a.js
import { b } from './b';
...

// b.js
export const b = 'foo';

And inspecting the node trying to apply the replaceWith operation I'm seeing: CallExpression as parent path for the working context with a valid container Node object while with the coverage flag context includes a SequenceExpression as the parent. Also, tried to add a failing test but seems to "work" in the plugin test context. I'm not using typescript or any other babel plugins for that matter (just 'react-app' preset) so I wonder if the transformations applied for TS are "solving" whatever might be causing this?

daetal-us commented 3 years ago

without coverage:

without-coverage

with coverage:

with-coverage
daetal-us commented 3 years ago

For posterity, the pastes above where output of the input Node's context for the success (w/o coverage) and failure (w/ coverage - triggering Container is falsy ReferenceError) of replaceWith calls against the same input as called from:

Xunnamius commented 3 years ago

@daetal-us Thank you for taking the time to analyze this in depth! Very interesting about attempting to add the failing test. The problem might be that the way I'm transforming the assignment expression is naive (I wish Babel had better documentation, hah). It might also be that SequenceExpression needs special handling. If, free time willing, you can throw together a MRE that fails as you describe, I can come up with a fix. I'm curious to see what the issue is :)

In the meantime, if it's only when calling replaceWith on the left property of assignment expressions that is causing trouble, perhaps it would be best to hide transforming assignment expressions behind a flag with default disabled. I don't think the transform is entirely necessary, and all other identifiers would still be transformed when encountered. Would this fix your use case?

Xunnamius commented 3 years ago

I don't think the transform is entirely necessary, and all other identifiers would still be transformed when encountered.

Of all the tests, this was the only difference between the current 1.0.0 functionality and 1.0.1 with the transformAssignExpr==false flag: Screenshot_20210430_160921

Not bad. I'll publish this along with more complete documentation.

Xunnamius commented 3 years ago

(I'll keep this open for now)

vzaidman commented 3 years ago

Hey! Thanks for the great library!

I worked around this issue using --coverageProvider=v8 instead of the default --coverageProvider=babel

jest --coverage --coverageReporters=cobertura --coverageProvider=v8

btw I published an article that might be of an interest to you where I mention this library: https://medium.com/welldone-software/jest-how-to-mock-a-function-call-inside-a-module-21c05c57a39f