react icon indicating copy to clipboard operation
react copied to clipboard

[Compiler Bug]: Can't handle identifiers that refer to both type and value

Open eps1lon opened this issue 1 year ago • 2 comments

What kind of issue is this?

  • [ ] React Compiler core (the JS output is incorrect, or your app works incorrectly after optimization)
  • [X] 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/#N4Igzg9grgTgxgUxALhASwLYAcIwC4AEwBAwhBhggHZ4BKCAhnHmhFQQL4EBmM5BAHRAA6APR4AnlgRghAbgFVFaGghjcmCAgAU+WMEUUECMRs1ZVkpcpRr0mLNgqodFi7lCrm21itTpmjlQAFMSmDhacVroQ+gCUhuwmCHiw7FRQADaZzq5UIBxAA

Repro steps

Apply React Compiler to

import { CommentReaction } from "./types";

interface Props {
  reaction: CommentReaction;
}

function CommentReaction({ reaction }: Props) {
  return null;
}

resulting JS issues TypeError: Duplicate declaration "CommentReaction" when passed through babel-plugin-react-compiler. The playground produces a build error.

This is easily fixable by changing the code to

-import { CommentReaction } from "./types";
+import { type CommentReaction } from "./types";

The original code is actually valid TypeScript (though I would not recommend authoring it this way). Despite CommentReaction being both a type and value, TypeScript is able to distinguish when it's used as a value vs type.

How often does this bug happen?

Every time

What version of React are you using?

19.0.0-beta-04b058868c-20240508

eps1lon avatar May 22 '24 22:05 eps1lon

Wow, I didn't realize that was even valid TypeScript. We currently use Babel to resolve identifiers during the Babel AST -> Compiler HIR conversion process so i'm wondering if maybe the issue is there. In either case, thanks for filing this! We'll take a look.

josephsavona avatar May 22 '24 22:05 josephsavona

Wow, I didn't realize that was even valid TypeScript.

Same reaction I always have when I stumble over this once a year in product code. But apparently this is supposed to be useful for something. Probably related to declaration merging. I thought there was a lint rule against this but I can't find it anymore.

It does work when the type isn't an import but explicitly declared in the file e.g.

-import { CommentReaction } from "./types";
+interface CommentReaction {}

eps1lon avatar May 22 '24 23:05 eps1lon

I guess this should not be a issue for the React Compiler since it don't transform typescript to javascript as @josephsavona mentioned. Also I found a fix from babel which says they only support duplication between imports with type annotations and local function.

And that's why add type before CommentReaction would a easy fix.

Fnll avatar Jun 09 '24 16:06 Fnll