-
Notifications
You must be signed in to change notification settings - Fork 49k
[compiler][gating] Experimental directive based gating #33149
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
Conversation
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.
How would this work for real code? If you use the use memo if(...)
form, what would we use as the source of the import?
For a large codebase internally, we added a
This codebase currently uses a lightweight wrapper around the gating config, but we can extend the same self-service gating function mechanism to We briefly discussed inserting more analyzable gating (e.g. Letting developers specify their own filenames (e.g. |
672e66b
to
4c48a87
Compare
const dynamicGating = findDirectivesDynamicGating(directives, opts); | ||
if (dynamicGating.isOk()) { | ||
return Ok(dynamicGating.unwrap()?.directive ?? null); | ||
} else { | ||
return Err(dynamicGating.unwrapErr()); | ||
} |
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.
Dynamic gating should also be an opt-in if compilationMode: "annotation"
is enabled.
If "use no memo"
is set at module scope, use memo if(...)
should have the same semantics as use memo
, whatever that is
} | ||
const functionGating = dynamicGating ?? pass.opts.gating; | ||
if (kind === 'original' && functionGating != null) { | ||
referencedBeforeDeclared ??= |
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.
not sure how we feel about ??=
when the right hand side is a potentially expensive computation. I'm happy to make this more explicit if that's preferred (e.g. if (referencedBeforeDeclared == null) ...
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.
i think this is fine, ??=
is a short-circuiting operator just like ||=
0013ccc
to
5b7cdf7
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.
nice!
} | ||
const functionGating = dynamicGating ?? pass.opts.gating; | ||
if (kind === 'original' && functionGating != null) { | ||
referencedBeforeDeclared ??= |
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.
i think this is fine, ??=
is a short-circuiting operator just like ||=
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
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)
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)
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>; ```
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
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)
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)
…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)
…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)
…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)
…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)
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)
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)
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)
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)
Adds
dynamicGating
as an experimental option for testing rollout DX at Meta. If specified, this enables dynamic gating which matchesuse memo if(...)
directives.Example usage
Input file
Compiler output
Stack created with Sapling. Best reviewed with ReviewStack.