Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Stop using React internals #419

Open
alexandernanberg opened this issue Sep 9, 2024 · 1 comment
Open

Stop using React internals #419

alexandernanberg opened this issue Sep 9, 2024 · 1 comment

Comments

@alexandernanberg
Copy link

🚀 Feature Proposal

Stop using React internals.

Motivation

https://x.com/sebmarkbage/status/1795892981976752210

Due to its heavy use of React internals it's likely to break even in minors.
However, because it creates a fake React Element object it also means that once that object gets into React's source code it'll deopt the JS VM.

Example

N/A

Pitch

I do really think fbt offers the best DX among the i18n libs out there. However when maintainers of React highly recommends against using it, it's a hard sell.

@kayhadrin
Copy link
Contributor

kayhadrin commented Sep 12, 2024

Thanks for bringing this topic to my attention!

I've commented on the Twitter thread:

[Replying to @sebmarkbage]: I'm not an expert on the topic of JIT deopt but I guess you're saying that when a React component tree contains FbtResult objects (that mimick React nodes), it's throwing off the JIT assumptions of the React internal code.

Back then, I was also promoting making fbt a proper React component...
Pros: proper type checks, more React-friendly, prevent i18n strings from being concatenated in the wild.
Cons: 2 ways to use fbt strings in a giant codebase.
That was never prioritized unfortunately.

Considering that facebook.com uses <fbt> already everywhere, it's hard to make a case that it's causing such a perf regression that it should never be used with React... Some benchmarking of using <fbt> in JSX vs returning the fbt result from a normal React component would be definitely appreciated.

Now, with regards to this "Stop using React internals" topic, I think this could be done like this for example:

  1. Customize the getFbtResult hook (from FbtHooks.js) so that it returns an array of React nodes.
    That's assuming that we can still directly use React.createElement() nowadays?
    We'd also need to inject the React module and some local variables via Babel transforms to do this automatically.
    E.g.
[ 
  'bonjour', 
  React.createElement(
    'strong',
    { className: 'greeting' },
    'le monde'
  )
]
  1. But at that point, since we're deep in the weeds using Babel transforms, we may as well just transform the <fbt> JSX to something that looks like proper React JSX and let the React JSX transform do its job normally. So let's say we make a new Fbt construct named <FbtComponent> to represent a React-friendly fbt string. Then, we would look for it in the AST and transform it to something that's already usable by the fbt._() client-side function.
    E.g. Before:
<Example>
  <FbtComponent desc="...">
    Hello
    <strong class="greeting">
      world
    </strong>
  </FbtComponent>
</Example>

After:

<Example>
  {
    // using array node and direct strings because <fbt> and JSX have different ways 
    // to interpret "white space" between text and JSX entities (if I recall correctly)
    [ 
      // 1. Pseudo-code for the client-side fbt._() translation payload for simplicity
      // 2. By now, fbt._() may only return plain strings
      fbt._('Bonjour'), 
      <strong class="greeting">
        {
          fbt._('le monde')
        }
      </strong>
    ]
  }
</Example>

This is just some quick mental rumbling from me today though! It's not entirely fleshed out but I hope it helps!

PS: I've moved on from the Meta i18n team a while ago but I'm still hovering over this repo now and then ;-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants