Skip to content

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Sep 24, 2025

We previously always generated import statements for any modules that had to be required, notably the import {c} from 'react/compiler-runtime' for the memo cache function. However, this obviously doesn't work when the source is using commonjs. Now we check the sourceType of the module and generate require() statements if the source type is 'script'.

I initially explored using https://babeljs.io/docs/babel-helper-module-imports, but the API design was unfortunately not flexible enough for our use-case. Specifically, our pipeline is as follows:

  • Compile individual functions. Generate candidate imports, pre-allocating the local names for those imports.
  • If the file is compiled successfully, actually add the imports to the program.

Ie we need to pre-allocate identifier names for the imports before we add them to the program — but that isn't supported by babel-helper-module-imports. So instead we generate our own require() calls if the sourceType is script.

@meta-cla meta-cla bot added the CLA Signed label Sep 24, 2025
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label Sep 24, 2025
@josephsavona josephsavona changed the title [commit] Add support for commonjs [compiler] Add support for commonjs Sep 24, 2025
@josephsavona josephsavona changed the title [compiler] Add support for commonjs [commit] Add support for commonjs Sep 24, 2025
We previously always generated import statements for any modules that had to be required, notably the `import {c} from 'react/compiler-runtime'` for the memo cache function. However, this obviously doesn't work when the source is using commonjs. Now we check the sourceType of the module and generate require() statements if the source type is 'script'.

I initially explored using https://babeljs.io/docs/babel-helper-module-imports, but the API design was unfortunately not flexible enough for our use-case. Specifically, our pipeline is as follows:
* Compile individual functions. Generate candidate imports, pre-allocating the local names for those imports.
* If the file is compiled successfully, actually add the imports to the program.

Ie we need to pre-allocate identifier names for the imports before we add them to the program — but that isn't supported by babel-helper-module-imports. So instead we generate our own require() calls if the sourceType is script.
@josephsavona josephsavona changed the title [commit] Add support for commonjs [compiler] Add support for commonjs Sep 24, 2025
Copy link
Member

@poteto poteto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edit: nvm

Copy link
Collaborator

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sweet, thank you! I also only found the one spot where the import specifier was generated so I believe this covers all. Will let you know asap once I tried that in Next.js if that fixes all the spots we noticed.

@josephsavona josephsavona merged commit 8ad773b into main Sep 24, 2025
21 of 30 checks passed
@josephsavona
Copy link
Member Author

Yeah, this should be the only spot we create imports. There is one place that we generate an export, but we only do that if the input also had an export so it should be fine in commonjs.

github-actions bot pushed a commit that referenced this pull request Sep 24, 2025
We previously always generated import statements for any modules that
had to be required, notably the `import {c} from
'react/compiler-runtime'` for the memo cache function. However, this
obviously doesn't work when the source is using commonjs. Now we check
the sourceType of the module and generate require() statements if the
source type is 'script'.

I initially explored using
https://babeljs.io/docs/babel-helper-module-imports, but the API design
was unfortunately not flexible enough for our use-case. Specifically,
our pipeline is as follows:
* Compile individual functions. Generate candidate imports,
pre-allocating the local names for those imports.
* If the file is compiled successfully, actually add the imports to the
program.

Ie we need to pre-allocate identifier names for the imports before we
add them to the program — but that isn't supported by
babel-helper-module-imports. So instead we generate our own require()
calls if the sourceType is script.

DiffTrain build for [8ad773b](8ad773b)
github-actions bot pushed a commit that referenced this pull request Sep 24, 2025
We previously always generated import statements for any modules that
had to be required, notably the `import {c} from
'react/compiler-runtime'` for the memo cache function. However, this
obviously doesn't work when the source is using commonjs. Now we check
the sourceType of the module and generate require() statements if the
source type is 'script'.

I initially explored using
https://babeljs.io/docs/babel-helper-module-imports, but the API design
was unfortunately not flexible enough for our use-case. Specifically,
our pipeline is as follows:
* Compile individual functions. Generate candidate imports,
pre-allocating the local names for those imports.
* If the file is compiled successfully, actually add the imports to the
program.

Ie we need to pre-allocate identifier names for the imports before we
add them to the program — but that isn't supported by
babel-helper-module-imports. So instead we generate our own require()
calls if the sourceType is script.

DiffTrain build for [8ad773b](8ad773b)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Sep 27, 2025
We previously always generated import statements for any modules that
had to be required, notably the `import {c} from
'react/compiler-runtime'` for the memo cache function. However, this
obviously doesn't work when the source is using commonjs. Now we check
the sourceType of the module and generate require() statements if the
source type is 'script'.

I initially explored using
https://babeljs.io/docs/babel-helper-module-imports, but the API design
was unfortunately not flexible enough for our use-case. Specifically,
our pipeline is as follows:
* Compile individual functions. Generate candidate imports,
pre-allocating the local names for those imports.
* If the file is compiled successfully, actually add the imports to the
program.

Ie we need to pre-allocate identifier names for the imports before we
add them to the program — but that isn't supported by
babel-helper-module-imports. So instead we generate our own require()
calls if the sourceType is script.

DiffTrain build for [8ad773b](facebook@8ad773b)
github-actions bot pushed a commit to code/lib-react that referenced this pull request Sep 27, 2025
We previously always generated import statements for any modules that
had to be required, notably the `import {c} from
'react/compiler-runtime'` for the memo cache function. However, this
obviously doesn't work when the source is using commonjs. Now we check
the sourceType of the module and generate require() statements if the
source type is 'script'.

I initially explored using
https://babeljs.io/docs/babel-helper-module-imports, but the API design
was unfortunately not flexible enough for our use-case. Specifically,
our pipeline is as follows:
* Compile individual functions. Generate candidate imports,
pre-allocating the local names for those imports.
* If the file is compiled successfully, actually add the imports to the
program.

Ie we need to pre-allocate identifier names for the imports before we
add them to the program — but that isn't supported by
babel-helper-module-imports. So instead we generate our own require()
calls if the sourceType is script.

DiffTrain build for [8ad773b](facebook@8ad773b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants