[Compiler Todo]: Handle TSSatisfiesExpression expressions

nikeee commented 4 weeks ago

What kind of issue is this?

[ ] React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
[ ] babel-plugin-react-compiler (build issue installing or using the Babel plugin)
[ ] eslint-plugin-react-compiler (build issue installing or using the eslint plugin)
[ ] react-compiler-healthcheck (build issue installing or using the healthcheck script)

Link to repro

https://playground.react.dev/#N4Igzg9grgTgxgUxALhHCA7MAXABAQ1wF5cBGXMfbASzADNqExcMoBbAIwRgG4AdDCAC+QA

Repro steps

The compiler currently cannot handle TSSatisfiesExpression

const a = 1 satisfies number;

Results in

(BuildHIR::lowerExpression) Handle TSSatisfiesExpression expressions

I think adding support to the code itself would be pretty straignt-forward, as the typescript construct of satisfies can just be ignored and handled as the inner expression:

    case "TSSatisfiesExpression": {
      const expr = exprPath as NodePath<t.TSSatisfiesExpression>;
      return lowerExpression(builder, expr.get("expression"));
    }

https://github.com/facebook/react/compare/main...nikeee:react:handle-satisfies-expression?expand=1

However, when implementing this, it does not build, due to type conflicts of @babel/types. npm ls @babel/types suggests that there are multiple versions of @babel/types in use, some of which already support satisfies and others don't. satisfies was introduced in TS 4.9, which is supported since babel 7.20.0. The RC itself does depend on a lower version, but ^7.20 versions are included as a transitive dependency. This might be the reason why the parser does not throw an error, while RC cannot handle it (and BuildHIR doesn't know anything about that). I also tried upgrading babel oin the entire project, but that seems to be not as trivial as I'd thought. Maybe there is a different solution.

How often does this bug happen?

Every time

What version of React are you using?

19

josephsavona commented 4 weeks ago

For some internal use-cases we need to support older versions of Babel, so unfortunately we can't just upgrade to 7.20. However we can align on a more recent version of @babel/types that does support this ast type, and then handle it as you suggested. Open to PRs!

josephsavona commented 4 weeks ago

Note that all errors categorized as todo are places where the compiler knowingly does not support a feature. We don't categorize those as bugs because the compiler is safely skipping compilation of something it can't compile. In general we're open to PRs to add support for additional syntax as long as its part of JS, TS, or Flow. There are some exceptions — for example, we probably won't ever support nested class declarations within components, bc the complexity cost is high enough and it's exceedingly rare. But for everday, uncomplicated things like sastisfies we're open to PRs.

rodrigofariow commented 3 weeks ago

For those that may want to try to "patch" this I used @nikeee gist and pnpm patched babel-plugin-react-compiler with it.

Everything seems to work and according to @josephsavona it should be mostly fine? Works for my project 🥳

Cheers

facebook / react