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
logical-assignment: Do not assign names to anonymous functions #11370
logical-assignment: Do not assign names to anonymous functions #11370
Conversation
arku
commented
Apr 2, 2020
•
edited
edited
Q | A |
---|---|
Fixed Issues? | Fixes #11362 |
Patch: Bug Fix? | Yes |
Major: Breaking Change? | No |
Minor: New Feature? | No |
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | |
License | MIT |
b626107
to
cbc73c0
Compare
A bunch of stuff can be introduced to make the function anonymous, the easiest ones would be:
var _f;
var f;
((_f = f) !== null && _f !== void 0 ? _f : f = [function () {}][0]).name;
function ensureAnonymous(fn) { return fn }
var _f;
var f;
((_f = f) !== null && _f !== void 0 ? _f : f = ensureAnonymous(function () {})).name; Both of those are valid es5 so I would consider them being better alternatives. I don't think that all current transforms (but I might be wrong) are that much concerned about |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We actually could update the spec to give anonymous closures a name. We don't have it for any of the math assignment operators, because the function just gets coerced into a number. I'll open an issue.
introduce a helper
Using a IIFE sounds good to me. I don't think we need a helper, though, it's not that much code.
packages/babel-plugin-proposal-logical-assignment-operators/src/index.js
Outdated
Show resolved
Hide resolved
packages/babel-plugin-proposal-logical-assignment-operators/src/index.js
Outdated
Show resolved
Hide resolved
tc39/proposal-logical-assignment#23 (comment) mentions that we can also use a // input
foo ??= function() {};
// output
foo ?? (foo = (0, function() {})); |
cbc73c0
to
fd88b0f
Compare
@jridgewell I have updated my PR with the suggested changes. Do you mind taking another look? |
packages/babel-plugin-proposal-logical-assignment-operators/src/index.js
Outdated
Show resolved
Hide resolved
...al-logical-assignment-operators/test/fixtures/logical-assignment/anonymous-function/input.js
Outdated
Show resolved
Hide resolved
fd88b0f
to
b09826c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
{ | ||
"plugins": [ | ||
"proposal-logical-assignment-operators", | ||
"proposal-nullish-coalescing-operator" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for reviewers: the "exec" and "transform" tests are in different folders because they have different configs.
packages/babel-plugin-proposal-logical-assignment-operators/src/index.js
Outdated
Show resolved
Hide resolved
@babel/babel Any idea what's going on with the tests? |
@JLHwung It is related to |
7997fb2
to
a17e23a
Compare
Rebased on upstream. The exports error should be fixed in recent releases. |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit a17e23a:
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/21810/ |
This is a partial revert of babel#11370, updating the tests to match the [new consensus](babel/proposals#66 (comment)).
This is a partial revert of #11370, updating the tests to match the [new consensus](babel/proposals#66 (comment)).