Use named imports for babel types by JLHwung · Pull Request #13685 · babel/babel

JLHwung · 2021-08-17T15:38:31Z

Q	A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

This PR is a follow-up to #13593 (review). In this PR we transform @babel/types namespace imports (e.g. import * as t from "@babel/types" to named imports (e.g. import { isIdentifier } from "@babel/types"). The named imports allow us to perform further optimization when transforming to commonjs modules.

Example

For example, before this PR, the named imports

import { isIdentifier } from "@babel/types";
isIdentifier(node)

are transformed as

const babel$types = require("@babel/types");
babel$types.isIdentifier(node)

After this PR, the named imports are transformed as

const _t = require("@babel/types");
const { isIdentifier } = _t
isIdentifier(node)

In #13593 we have identified a bottleneck of generator performance, accessing methods from @babel/types namespace is very slow. The optimized output now accesses namespace only once (per module).

Note that we can not generalize the optimization to any imports because ES imports are live bindings, which means if a module changed its exports on-the-fly, the updated values will be synchronized to the consumers. However, in the transpiled code, we takes a snapshot of these imports once and they won't be updated since then.

This approach is safe for most @babel/types usage in our current codebase since we don't modify the types exports, except in babel-traverse/src/path/index.ts where @babel/traverse is modifying the exports of @babel/types. Such usage is rare and we should consider if we should move on to a new approach.

The first commit is produced by a custom codemod.

Comparison to plugin-only approach

My first draft is a plugin transforming namespace import * as t to destructuring. Compared to current approach, the first approach has the following constraints:

Hard to determine which packages should be published because changes are only in build settings.
A valid method usage like const addComment = t.addComment becomes invalid after built, though we could avoid naming conflicts by sacrificing code readability.

Limitations

This PR does not change the indirect @babel/types usage such as

Use @babel/types from @babel/core

import { types as t } from "@babel/core"

Use @babel/types from the callback parameters of a plugin

module.exports = function({ types: t }) {
  
}

I think they should be updated, too. I will address them in separate PRs.

Further thoughts

Should we come up with a parameterized (🤷) assumption for commonjs transform?

{
  noImportLiveBindings: ["@babel/types"]
}

babel-bot · 2021-08-17T15:41:12Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/48210/

codesandbox-ci · 2021-08-17T15:43:09Z

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 d9c6681:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

hzoo

👍 codemod approach makes the most sense and separating out follow up PRs, nice work!

lightmare · 2021-08-18T19:49:30Z

I'm curious: is the slowdown due to the "emulation" of ES exports via getters? If that's the case, perhaps the issue could be alleviated at the source, i.e. in @babel/types by exporting fixed values instead.

Here's an idea: lightmare@5842734
It makes babel-helper-module-transforms act as if constantReexports = true when the source contains custom directive "@babel noLiveExports". Putting that directive into babel-types/src/index.ts then re-exports values, rather than getters.
What I haven't figured out yet, is how to remove the directive when exports are not being transpiled into requires.

JLHwung · 2021-08-18T20:13:18Z

is the slowdown due to the "emulation" of ES exports via getters?

That makes sense to me.

If that's the case, perhaps the issue could be alleviated at the source, i.e. in @babel/types by exporting fixed values instead.

Changing the exported values may break use cases such to manipulate the @babel/types exports and then to reuse it later. We can only do it on the consumer side when we are sure we don't need the live bindings. The good news is that when Babel 8 is published in ESM, it is likely we don't need this plugin, unless the native ESM live bindings have bad performance.

lightmare · 2021-08-18T21:11:10Z

Changing the exported values may break use cases such to manipulate the @babel/types exports and then to reuse it later.

Not sure what you mean. Exports defined with Object.defineProperty(exports, key, { enumerable: true, get() { ... } } ) cannot be manipulated:

re-assignment fails silently
re-defineProperty fails loudly

Although that wasn't my intent, changing the exports to plain assignment (hence configurable: true) makes that kind of manipulation possible. I don't think doing that would be a good idea, though. Because all the other functions that @babel/types is re-exporting from submodules will still be referencing the original functions — individual submodules are importing directly from other submodules.

JLHwung added 2 commits August 17, 2021 11:24

migrate to named babel types imports

1e89843

perf: transform babel types import to destructuring

8e76e93

JLHwung added the PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories label Aug 17, 2021

JLHwung added 2 commits August 17, 2021 11:59

fix merge errors

527d4b3

apply plugin to itself

d9c6681

hzoo approved these changes Aug 17, 2021

View reviewed changes

existentialism approved these changes Aug 17, 2021

View reviewed changes

JLHwung merged commit 614b486 into babel:main Aug 18, 2021

JLHwung deleted the use-named-import-for-babel-types branch August 18, 2021 14:28

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Nov 18, 2021

github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use named imports for babel types#13685

Use named imports for babel types#13685
JLHwung merged 4 commits intobabel:mainfrom
JLHwung:use-named-import-for-babel-types

JLHwung commented Aug 17, 2021 •

edited

Loading

Uh oh!

babel-bot commented Aug 17, 2021 •

edited

Loading

Uh oh!

codesandbox-ci bot commented Aug 17, 2021 •

edited

Loading

Uh oh!

hzoo left a comment

Uh oh!

lightmare commented Aug 18, 2021 •

edited

Loading

Uh oh!

JLHwung commented Aug 18, 2021

Uh oh!

lightmare commented Aug 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

JLHwung commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example

Comparison to plugin-only approach

Limitations

Further thoughts

Uh oh!

babel-bot commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codesandbox-ci bot commented Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hzoo left a comment

Choose a reason for hiding this comment

Uh oh!

lightmare commented Aug 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JLHwung commented Aug 18, 2021

Uh oh!

lightmare commented Aug 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

JLHwung commented Aug 17, 2021 •

edited

Loading

babel-bot commented Aug 17, 2021 •

edited

Loading

codesandbox-ci bot commented Aug 17, 2021 •

edited

Loading

lightmare commented Aug 18, 2021 •

edited

Loading