🚀 Transform Function Declarations #23358
Conversation
|
|
||
| const BAIL_OUT_CONDITIONS = { | ||
| // If this isn't a FunctionDeclaration, bail out on modification. | ||
| isNotFunction: (t, path) => !t.isFunctionDeclaration(path.node), |
There was a problem hiding this comment.
Why are you ignoring function expressions?
There was a problem hiding this comment.
Scope, starting with a very small surface. Happy to extend though if this is too restrictive.
build-system/babel-plugins/babel-plugin-transform-function-declarations/index.js
Outdated
Show resolved
Hide resolved
build-system/babel-plugins/babel-plugin-transform-function-declarations/index.js
Outdated
Show resolved
Hide resolved
build-system/babel-plugins/babel-plugin-transform-function-declarations/index.js
Show resolved
Hide resolved
build-system/babel-plugins/babel-plugin-transform-function-declarations/index.js
Show resolved
Hide resolved
|
|
||
| // If the FunctionDeclaration identifier is newed in the scope of this program, bail out on modification. | ||
| isNewedInProgramScope: (t, path) => { | ||
| const methodName = path.get('id').node.name; |
There was a problem hiding this comment.
Use path.scope.getBinding(name) to quickly find all references to the identifier.
There was a problem hiding this comment.
I find the current syntax far more expressive of it's intent. Is there a strong push here to change?
There was a problem hiding this comment.
It's like a bajillion times faster for large files.
build-system/babel-plugins/babel-plugin-transform-function-declarations/index.js
Outdated
Show resolved
Hide resolved
|
Neat. I'm curious what the expected benefits are (binary size?) and why we don't just change the source and add a linter to prefer arrows over functions. Found out last week that this is recommended by Google's style guide too:
|
|
The goal is a binary size savings for the Since this transform is mostly lossless, it could only be applied on the |
| .log( | ||
| `Bail on ${ | ||
| path.get('id') && path.get('id').node.name | ||
| ? `function ${path.node.id.name}:` |
There was a problem hiding this comment.
Could we add a message string function in conjunction with "avoid else, return/continue early" to prevent Hadouken nesting? 😬
There was a problem hiding this comment.
Sure thing, makes sense.
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| const thing = async (a, b) => a + b; |
There was a problem hiding this comment.
nit: newlines in test files.
|
@alanorozco is going to take this PR and drive to completion. Thanks Alan for taking this on! |
|
@kristoferbaxter What's the current state? Is the plugin ready but depending on the post-Closure Babel step? EDIT: Cleared up offline. Remaining tasks:
Will apply optimization but disable the plugin. Then I'll include it in the post-Closure step. I'll see if I can work on this PR directly, if not I'll fork it. |
|
Thanks @alanorozco – sorry for missing your question here and thanks for following up offline. |
|
Closed in favor of #26779 |
Babel Plugin that safely converts function declaration to arrow function declarations, where possible losslessly.
compiles to
This PR does not enable the plugin, as we want to ship Babel/Multipass before testing.