Skip to content

Conversation

josephsavona
Copy link
Member

@josephsavona josephsavona commented Jun 14, 2024

Stack from ghstack (oldest at bottom):

Adds fixtures for macro.namespace(...) style invocations which we use internally in some cases instead of just macro(...). I tried every example i could think of that could possibly break it (including basing one off of another fixture where we hit an invariant related due to a temporary being emitted for a method call), and they all worked. I just had to fix an existing bug where we early return in some cases instead of continuing, which is a holdover from when this pass was originally written as a ReactiveFunction visitor.

Copy link

vercel bot commented Jun 14, 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 14, 2024 3:46pm

josephsavona added a commit that referenced this pull request Jun 14, 2024
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 14, 2024
@react-sizebot
Copy link

react-sizebot commented Jun 14, 2024

Comparing: a07f5a3...b223c3a

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 = 1.82 kB 1.82 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.80 kB 497.80 kB = 89.24 kB 89.24 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.67 kB 6.67 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.62 kB 502.62 kB = 89.94 kB 89.94 kB
facebook-www/ReactDOM-prod.classic.js = 597.04 kB 597.04 kB = 105.31 kB 105.31 kB
facebook-www/ReactDOM-prod.modern.js = 571.38 kB 571.38 kB = 101.25 kB 101.25 kB
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 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 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Generated by 🚫 dangerJS against b223c3a

Adds fixtures for `macro.namespace(...)` style invocations which we use internally in some cases instead of just `macro(...)`. I tried every example i could think of that could possibly break it (including basing one off of another fixture where we hit an invariant related due to a temporary being emitted for a method call), and they all worked. I just had to fix an existing bug where we early return in some cases instead of continuing, which is a holdover from when this pass was originally written as a ReactiveFunction visitor.

[ghstack-poisoned]
josephsavona added a commit that referenced this pull request Jun 14, 2024
Adds fixtures for `macro.namespace(...)` style invocations which we use internally in some cases instead of just `macro(...)`. I tried every example i could think of that could possibly break it (including basing one off of another fixture where we hit an invariant related due to a temporary being emitted for a method call), and they all worked. I just had to fix an existing bug where we early return in some cases instead of continuing, which is a holdover from when this pass was originally written as a ReactiveFunction visitor.

ghstack-source-id: c01f45b
Pull Request resolved: #29899
@josephsavona josephsavona requested a review from mofeiZ June 14, 2024 18:06
Copy link
Contributor

@mofeiZ mofeiZ left a comment

Choose a reason for hiding this comment

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

Ahh makes sense, nice fix!

@josephsavona josephsavona merged commit b223c3a into gh/josephsavona/32/base Jun 17, 2024
josephsavona added a commit that referenced this pull request Jun 17, 2024
Adds fixtures for `macro.namespace(...)` style invocations which we use internally in some cases instead of just `macro(...)`. I tried every example i could think of that could possibly break it (including basing one off of another fixture where we hit an invariant related due to a temporary being emitted for a method call), and they all worked. I just had to fix an existing bug where we early return in some cases instead of continuing, which is a holdover from when this pass was originally written as a ReactiveFunction visitor.

ghstack-source-id: c01f45b
Pull Request resolved: #29899
@josephsavona josephsavona deleted the gh/josephsavona/32/head branch June 17, 2024 15:17
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.

4 participants