Skip to content

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jun 6, 2024

Stack from ghstack (oldest at bottom):

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]
Copy link

vercel bot commented Jun 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2024 7:16pm

@react-sizebot
Copy link

react-sizebot commented Jun 6, 2024

Comparing: a0a435d...f558784

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.66 kB 6.66 kB +0.05% 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.25 kB 497.25 kB = 89.11 kB 89.11 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.07 kB 502.07 kB = 89.79 kB 89.80 kB
facebook-www/ReactDOM-prod.classic.js = 596.75 kB 596.75 kB = 105.19 kB 105.19 kB
facebook-www/ReactDOM-prod.modern.js = 570.93 kB 570.93 kB = 101.13 kB 101.13 kB
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 63.89 kB 0.00 kB Deleted 15.97 kB 0.00 kB

Generated by 🚫 dangerJS against f558784

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Jun 6, 2024
There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

ghstack-source-id: 91ba77d
Pull Request resolved: #29781
Copy link

@victor-0x29a victor-0x29a left a comment

Choose a reason for hiding this comment

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

some adjust

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]
There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]
Copy link
Contributor

@gsathya gsathya left a comment

Choose a reason for hiding this comment

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

yeah this makes sense

There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

[ghstack-poisoned]
@josephsavona josephsavona merged commit f558784 into gh/josephsavona/22/base Jun 10, 2024
josephsavona added a commit that referenced this pull request Jun 10, 2024
There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

ghstack-source-id: aea6dab
Pull Request resolved: #29781
@josephsavona josephsavona deleted the gh/josephsavona/22/head branch June 10, 2024 15:42
Testmasha pushed a commit to Testmasha/Test that referenced this pull request Apr 15, 2025
There are a few places where we want to check whether a value actually got memoized, and we currently have to infer this based on values that "should" have a scope and whether a corresponding scope actually exists. This PR adds a new ReactiveStatement variant to model a reactive scope block that was pruned for some reason, and updates all the passes that prune scopes to instead produce this new variant.

ghstack-source-id: 91ba77d
Pull Request resolved: facebook/react#29781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Component: React Compiler 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.

5 participants