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

[regression] [5.1.2] variable override #201

Closed
pi0 opened this issue Apr 22, 2019 · 3 comments
Closed

[regression] [5.1.2] variable override #201

pi0 opened this issue Apr 22, 2019 · 3 comments

Comments

@pi0
Copy link

pi0 commented Apr 22, 2019

This issue introduced with 5.1.2 (5.1.1 and below are fine)

Reproduction repo: https://github.com/pi0/babel-plugin-istanbul-bug (Use yarn test)

Reverting this change by #192 fixes problem.

Details

When using @babel/preset-env:

import path from 'path'

function test() {
  const _path = 'test'
  return path.relative(__dirname, _path)
}

Transpiles into: (Correct)

var _path2 = _interopRequireDefault(require("path"));

function test() {
  const _path = 'test';
  return _path2.default.relative(__dirname, _path);
}

Using babel-plugin-istanbul@5.1.2: (Bug)

var _path = _interopRequireDefault(require("path"));

function test() {
  cov_1523mz691.f[0]++;

  const _path = (cov_1523mz691.s[0]++, 'test');

  cov_1523mz691.s[1]++;
  return _path.default.relative(__dirname, _path);
}

Using babel-plugin-istanbul@5.1.1 (Correct)

var _path2 = _interopRequireDefault(require("path"));

function test() {
  cov_1523mz691.f[0]++;

  const _path = (cov_1523mz691.s[0]++, 'test');

  cov_1523mz691.s[1]++;
  return _path2.default.relative(__dirname, _path);
}
@coreyfarrell
Copy link
Member

I was able to further isolate this issue with the provided repository by replacing @babel/preset-env with @babel/plugin-transform-modules-commonjs. What I don't know yet is if we've exposed a bug in the babel ESM -> CJS transform plugin, or if it's inappropriate for us to call path.scope.crawl().

@leebyron unfortunately we may have to revert your patch. I did a quick experiment and was able to get babel-plugin-lodash working with babel-plugin-istanbul@5.1.1 by adding path.scope.crawl() to the end of the ImportDeclaration.exit() function at https://github.com/lodash/babel-plugin-lodash/blob/7d2342a3b849c3c8ef3b2d904b7e2394205466ff/src/index.js#L84. I'm not sure if this change is actually appropriate or just shifts the issue over there.

@coreyfarrell
Copy link
Member

I've just pushed changes to revert #192 and add a regression test to verify that babel-plugin-istanbul and @babel/plugin-transform-modules-commonjs play nice together. I'll try to push an updated release shortly.

@leebyron until we can find a new solution to #192 you will need to pin your dependency to babel-plugin-istanbul@5.1.2.

@coreyfarrell
Copy link
Member

babel-plugin-istanbul@5.1.3 is now released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants