Skip to content
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

fix(transpiler): remove react element inlining #13694

Merged
merged 3 commits into from
Sep 9, 2024
Merged

Conversation

paperdave
Copy link
Member

What does this PR do?

Fixes #11025

React 19 changed their element format in facebook/react#28813 to use a different symbol, as a warning that

  • the underlying element format has changed
  • the underlying format is going to receive more changes

It seems that inlining as we do it will not be a viable strategy with going forward, especially seeing Sebastian's comments on the subject.

I mean those are either broken or causing the page to deopt so you probably want to change them ASAP.

In addition to changing the format, we plan on JSX in RSC to have side-effects so the inlining strategy won't work in RSC regardless. So just upgrading an inlining plugin to use the new format might not be a good idea.

This changeset is tested by bundler_jsx ensuring that the symbol used in react/jsx-runtime is used instead of react.element. This is something I had already written a test for, but previously it was special cased to the inlined react.element.

@robobun
Copy link

robobun commented Sep 2, 2024

@Jarred-Sumner, your commit d524f88 has 12 failures in #3027:

  • test/integration/expo-app/expo.test.ts - 1 failing on 🪟 x64
  • test/integration/expo-app/expo.test.ts - 1 failing on 🪟 x64-baseline
  • test/cli/watch/watch.test.ts - timeout on 🪟 x64
  • test/cli/watch/watch.test.ts - timeout on 🪟 x64-baseline
  • test/cli/install/bun-run.test.ts - 1 failing on 🪟 x64
  • test/regression/issue/07500/07500.test.ts - 1 failing on 🪟 x64
  • test/cli/hot/watch.test.ts - 1 failing on 🪟 x64
  • test/cli/hot/watch.test.ts - 1 failing on 🪟 x64-baseline
  • test/js/node/http/node-http.test.ts - 2 failing on 🪟 x64-baseline
  • test/cli/hot/hot.test.ts - 1 failing on 🪟 x64
  • test/cli/hot/hot.test.ts - 2 failing on 🪟 x64-baseline
  • test/js/node/net/node-net-server.test.ts - 1 failing on 🪟 x64
  • test/regression/issue/ctrl-c.test.ts - 1 failing on 🍎 13 aarch64
  • test/bundler/bun-build-api.test.ts - 1 failing on 🪟 x64
  • test/bundler/bun-build-api.test.ts - 1 failing on 🪟 x64-baseline
  • Failed to create agent for 🐧 22.04 x64-baseline - test-bun
  • VcpuLimitExceededYou have requested more vCPU capacity than your current vCPU limit of 992 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.df1fe59f-4f13-46cc-8732-c7e09315bcc8
  • Failed to create agent for 🐧 20.04 x64-baseline - test-bun
  • VcpuLimitExceededYou have requested more vCPU capacity than your current vCPU limit of 992 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.5b4f6abe-17eb-4567-a4bb-ccc9b131d309
  • Failed to create agent for 🐧 20.04 x64-baseline - test-bun
  • VcpuLimitExceededYou have requested more vCPU capacity than your current vCPU limit of 992 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.cf3be745-9721-4c78-841c-68302cf6c8e2
  • Failed to create agent for 🐧 22.04 x64-baseline - test-bun
  • VcpuLimitExceededYou have requested more vCPU capacity than your current vCPU limit of 992 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.a4bc9282-64b5-4ab3-b216-0dc8b36c266a
  • Failed to create agent for 🐧 22.04 x64-baseline - test-bun
  • VcpuLimitExceededYou have requested more vCPU capacity than your current vCPU limit of 992 allows for the instance bucket that the specified instance type belongs to. Please visit http://aws.amazon.com/contact-us/ec2-request to request an adjustment to this limit.1731997f-0711-4995-825f-66707f5fb8d0
  • test/cli/install/registry/bun-install-registry.test.ts - 1 failing on 🍎 14 aarch64
  • test/cli/install/registry/bun-install-registry.test.ts - 3 failing on 🪟 x64
  • test/cli/install/registry/bun-install-registry.test.ts - 3 failing on 🪟 x64-baseline
  • @Jarred-Sumner
    Copy link
    Collaborator

    This was a 30% performance boost to React SSR. I do think we should expect to bring this back in some form.

    @paperdave paperdave merged commit d38f937 into main Sep 9, 2024
    44 of 45 checks passed
    @paperdave paperdave deleted the dave/disable-jsx-inline branch September 9, 2024 22:03
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    None yet
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    React 19 SSR Fails In Production Mode, But Works In Development
    3 participants