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

eps1lon commented 3 months ago

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

josephsavona commented 3 months ago

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.

eps1lon commented 3 months ago

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 {}

Fnll commented 3 months ago

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.

facebook / react