4Catalyzer / babel-plugin-dev-expression

A mirror of Facebook's dev-expression Babel plugin
MIT License
184 stars 6 forks source link

Transformation issue with es2015 #17

Open chemoish opened 4 years ago

chemoish commented 4 years ago

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 commented 4 years ago

@jquense You pointed this out a while ago in Slack. I'm confused why this is happening... they're both using node.callee here: https://github.com/4Catalyzer/babel-plugin-dev-expression/blob/c734a08b7f3fefc3e7479240013a4d9f907f4429/dev-expression.js#L79-L88

jquense commented 4 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

https://github.com/4Catalyzer/babel-plugin-dev-expression/blob/c734a08b7f3fefc3e7479240013a4d9f907f4429/dev-expression.js#L79-L88

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.