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

Transformation issue with es2015 #17

Open
chemoish opened this issue Jan 7, 2020 · 5 comments
Open

Transformation issue with es2015 #17

chemoish opened this issue Jan 7, 2020 · 5 comments

Comments

@chemoish
Copy link

chemoish commented Jan 7, 2020

I noticed that my commonjs build included the incorrectly transformed invariant(false).

This is incorrect as the import is being transformed to var _invariant and invariant isn't defined.

The issue can be reproduced in babel repl.

Not sure if invariant(false) can be converted to (0, _invariant.default)(false) or whatever.

@taion
Copy link
Contributor

taion commented Jan 10, 2020

@jquense You pointed this out a while ago in Slack. I'm confused why this is happening... they're both using node.callee here:

var devInvariant = t.callExpression(
node.callee,
[t.booleanLiteral(false)].concat(node.arguments.slice(1))
);
devInvariant[SEEN_SYMBOL] = true;
var prodInvariant = t.callExpression(
node.callee,
[t.booleanLiteral(false)]
);
prodInvariant[SEEN_SYMBOL] = true;

@jquense
Copy link
Member

jquense commented Jan 10, 2020

It might be that it just messes with the scope/binding info enough that it doesn't connect them anymore. Or maybe it tracks through the callExpression, not the callee?
Seems like just swapping out the arguments instead of the whole expression might fix it

@leepowelldev
Copy link

Did anyone find a solution to this, I'm hitting the same issue:

This:

import invariant from 'tiny-invariant';

invariant(children, 'Error!');

Results in:

!children ? process.env.NODE_ENV !== "production" ? (0, _tinyInvariant["default"])(false, 'Error!') : invariant(false) : void 0;

Where invariant is no longer imported.

@leepowelldev
Copy link

Bit more digging, and this seems to be caused by using @babel/preset-env - so I'm not sure it's a bug with this plugin

@JLHwung
Copy link

JLHwung commented Oct 14, 2021

var devInvariant = t.callExpression(
node.callee,
[t.booleanLiteral(false)].concat(node.arguments.slice(1))
);
devInvariant[SEEN_SYMBOL] = true;
var prodInvariant = t.callExpression(
node.callee,
[t.booleanLiteral(false)]
);
prodInvariant[SEEN_SYMBOL] = true;

The plugin reuses node.callee in the transformed AST. Babel can not tell they are actually different AST nodes, so only one of them is replaced by commonjs transform. Use t.cloneNode(node.callee) for prodInvariant will fix this issue: In my local repo I can not reproduce this issue with the suggested fix.

Duplicate AST nodes cause nasty issues like this, as a plugin author, you can use babel-check-duplicated-nodes to ensure the transformed AST does not contain duplicate nodes. Babel uses it, too.

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

5 participants