Skip to content

[compiler][entrypoint] Fix edgecases for noEmit and opt-outs #33148

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

Merged
merged 1 commit into from
May 9, 2025
Merged

Conversation

mofeiZ
Copy link
Contributor

@mofeiZ mofeiZ commented May 7, 2025

Comment on lines 109 to 110
this.hasModuleScopeOptOut =
findDirectiveDisablingMemoization(program.node.directives) != null;
Copy link
Member

Choose a reason for hiding this comment

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

It's much easier to reason about constructors when they are dumb data initialization, but this is moderately interesting program analysis. Maybe move this to be passed to the constructor as an argument?

Comment on lines 539 to 550
/**
* Always compile functions with opt in directives.
*/
Copy link
Member

Choose a reason for hiding this comment

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

this is outdated/wrong now

@mofeiZ mofeiZ force-pushed the pr33148 branch 5 times, most recently from 64f7bdd to bb0807a Compare May 8, 2025 18:57
@mofeiZ mofeiZ marked this pull request as ready for review May 8, 2025 18:59
@mofeiZ mofeiZ force-pushed the pr33148 branch 2 times, most recently from e247982 to 6d07fd0 Compare May 8, 2025 23:09
mofeiZ added a commit that referenced this pull request May 9, 2025
React Compiler's program traversal logic is pretty lengthy and complex
as we've added a lot of features piecemeal. `compileProgram` is 300+
lines long and has confusing control flow (defining helpers inline,
invoking visitors, mutating-asts-while-iterating, mutating global
`ALREADY_COMPILED` state).

- Moved more stuff to `ProgramContext`
- Separated `compileProgram` into a bunch of helpers

Tested by syncing this stack to a Meta codebase and observing no
compilation output changes (D74487851, P1806855669, P1806855379)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33147).
* #33149
* #33148
* __->__ #33147
github-actions bot pushed a commit that referenced this pull request May 9, 2025
React Compiler's program traversal logic is pretty lengthy and complex
as we've added a lot of features piecemeal. `compileProgram` is 300+
lines long and has confusing control flow (defining helpers inline,
invoking visitors, mutating-asts-while-iterating, mutating global
`ALREADY_COMPILED` state).

- Moved more stuff to `ProgramContext`
- Separated `compileProgram` into a bunch of helpers

Tested by syncing this stack to a Meta codebase and observing no
compilation output changes (D74487851, P1806855669, P1806855379)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33147).
* #33149
* #33148
* __->__ #33147

DiffTrain build for [5069e18](5069e18)
github-actions bot pushed a commit that referenced this pull request May 9, 2025
React Compiler's program traversal logic is pretty lengthy and complex
as we've added a lot of features piecemeal. `compileProgram` is 300+
lines long and has confusing control flow (defining helpers inline,
invoking visitors, mutating-asts-while-iterating, mutating global
`ALREADY_COMPILED` state).

- Moved more stuff to `ProgramContext`
- Separated `compileProgram` into a bunch of helpers

Tested by syncing this stack to a Meta codebase and observing no
compilation output changes (D74487851, P1806855669, P1806855379)
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33147).
* #33149
* #33148
* __->__ #33147

DiffTrain build for [5069e18](5069e18)
@mofeiZ mofeiZ merged commit 3820740 into main May 9, 2025
20 checks passed
github-actions bot pushed a commit that referenced this pull request May 9, 2025
Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* #33149
* __->__ #33148

DiffTrain build for [3820740](3820740)
github-actions bot pushed a commit that referenced this pull request May 9, 2025
Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* #33149
* __->__ #33148

DiffTrain build for [3820740](3820740)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 9, 2025
…k#33148)

Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* facebook#33149
* __->__ facebook#33148

DiffTrain build for [3820740](facebook@3820740)
github-actions bot pushed a commit to nikeee/react that referenced this pull request May 9, 2025
…k#33148)

Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* facebook#33149
* __->__ facebook#33148

DiffTrain build for [3820740](facebook@3820740)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 9, 2025
…k#33148)

Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* facebook#33149
* __->__ facebook#33148

DiffTrain build for [3820740](facebook@3820740)
github-actions bot pushed a commit to nikeee/react that referenced this pull request May 9, 2025
…k#33148)

Title
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33148).
* facebook#33149
* __->__ facebook#33148

DiffTrain build for [3820740](facebook@3820740)
mofeiZ added a commit that referenced this pull request May 21, 2025
Adds `dynamicGating` as an experimental option for testing rollout DX at
Meta. If specified, this enables dynamic gating which matches `use memo
if(...)` directives.

#### Example usage
Input file
```js
// @dynamicGating:{"source":"myModule"}
export function MyComponent() {
  'use memo if(isEnabled)';
   return <div>...</div>;
}
```
Compiler output
```js
import {isEnabled} from 'myModule';
export const MyComponent = isEnabled()
  ? <optimized version>
  : <original version>;
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33149).
* __->__ #33149
* #33148
github-actions bot pushed a commit that referenced this pull request May 21, 2025
Adds `dynamicGating` as an experimental option for testing rollout DX at
Meta. If specified, this enables dynamic gating which matches `use memo
if(...)` directives.

#### Example usage
Input file
```js
// @dynamicGating:{"source":"myModule"}
export function MyComponent() {
  'use memo if(isEnabled)';
   return <div>...</div>;
}
```
Compiler output
```js
import {isEnabled} from 'myModule';
export const MyComponent = isEnabled()
  ? <optimized version>
  : <original version>;
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33149).
* __->__ #33149
* #33148

DiffTrain build for [459a2c4](459a2c4)
github-actions bot pushed a commit that referenced this pull request May 21, 2025
Adds `dynamicGating` as an experimental option for testing rollout DX at
Meta. If specified, this enables dynamic gating which matches `use memo
if(...)` directives.

#### Example usage
Input file
```js
// @dynamicGating:{"source":"myModule"}
export function MyComponent() {
  'use memo if(isEnabled)';
   return <div>...</div>;
}
```
Compiler output
```js
import {isEnabled} from 'myModule';
export const MyComponent = isEnabled()
  ? <optimized version>
  : <original version>;
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33149).
* __->__ #33149
* #33148

DiffTrain build for [459a2c4](459a2c4)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 22, 2025
Adds `dynamicGating` as an experimental option for testing rollout DX at
Meta. If specified, this enables dynamic gating which matches `use memo
if(...)` directives.

#### Example usage
Input file
```js
// @dynamicGating:{"source":"myModule"}
export function MyComponent() {
  'use memo if(isEnabled)';
   return <div>...</div>;
}
```
Compiler output
```js
import {isEnabled} from 'myModule';
export const MyComponent = isEnabled()
  ? <optimized version>
  : <original version>;
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33149).
* __->__ facebook#33149
* facebook#33148

DiffTrain build for [459a2c4](facebook@459a2c4)
github-actions bot pushed a commit to code/lib-react that referenced this pull request May 22, 2025
Adds `dynamicGating` as an experimental option for testing rollout DX at
Meta. If specified, this enables dynamic gating which matches `use memo
if(...)` directives.

#### Example usage
Input file
```js
// @dynamicGating:{"source":"myModule"}
export function MyComponent() {
  'use memo if(isEnabled)';
   return <div>...</div>;
}
```
Compiler output
```js
import {isEnabled} from 'myModule';
export const MyComponent = isEnabled()
  ? <optimized version>
  : <original version>;
```
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33149).
* __->__ facebook#33149
* facebook#33148

DiffTrain build for [459a2c4](facebook@459a2c4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants