Support opt-in restrictions on 'module' and 'exports' usage alongside ES6 import/export. #6309
Conversation
loganfsmyth
force-pushed
the
assert-module-exports-usage
branch
2 times, most recently
from
3832842
to
a122577
Compare
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/5061/ |
| const { scope, bindingNames } = this; | ||
|
|
||
| const arg = path.get("argument"); | ||
| if (!arg.isIdentifier()) return; |
There was a problem hiding this comment.
This would be super (😉, you'll get it in a second) helpful for super.prop++ and this.#foo++, too.
There was a problem hiding this comment.
At the moment this only accepts a list of identifier names, so that'd be tough, but we could certainly expand that functionality later.
| ) { | ||
| // ++i => (i += 1); | ||
| path.replaceWith( | ||
| t.assignmentExpression("+=", arg.node, t.numericLiteral(1)), |
There was a problem hiding this comment.
Can we transform into a get and set? super.prop and this.#foo again.
There was a problem hiding this comment.
It's just translating into this to avoid code duplication. The AssignmentExpression visitor should still run on cases like this after this replacement happens, AFAIK, which will replace it with a get and a set.
| @@ -0,0 +1,22 @@ | |||
| import "foo"; | |||
There was a problem hiding this comment.
Do we have a test with a locally defined exports?
There was a problem hiding this comment.
I don't think we do. I'll add some.
loganfsmyth
force-pushed
the
assert-module-exports-usage
branch
from
a122577
to
e65994e
Compare
| @@ -0,0 +1,18 @@ | |||
| # babel-helper-simple-assignment | |||
|
|
|||
| There are many cases where it is hard to perform transformations because a | |||
There was a problem hiding this comment.
code using -> code is using
|
|
||
| It is difficult to work with. | ||
|
|
||
| This helper can handle converting these to simple access patterns of |
| `); | ||
| const exportsAssertion = template(` | ||
| (function(){ | ||
| throw new Error("The CommonJS 'exports' variable is not available in ES6 modules."); |
There was a problem hiding this comment.
This is also the case for __dirname, __filename, require et al but we can't really ban those until node gets a standard import.meta shape
There was a problem hiding this comment.
Yeah I'd be up for doing this for those too, but we can always do it behind a separate flag. For now at least this PR allows us to get closer.
lock
bot
added
the
outdated
Not breaking since it is opt in, but we should probably make this the default once we settle on a plan for #6242.
This injects code to throw an exception if you try to use
moduleorexportsinside an ES6 module. Webpack already does this, so hopefully it won't be too much of an upgrade issue.