Issue with named exports and filenames

[EmilNordling]

Hi!

Been lurking over the progress of this package for a while now and you've done amazing work to implement fast refresh for TypeScript, a true game changer! I haven't found time to a try it out until recently and these where the issues I ran into:

• If a module is exporting its React component as named instead of default, it'll throw error.
• If a module is not named exactly as its default exported React component, it'll not recover state.

In a Babel setup, it is working to have both named export and non-matching filenames to the component.

If I weren't too unfamiliar with custom ts transformers, I'd try to create a PR! But I don't have that experience unfortunately, what I can do is provide a branch where these errors occurd in: https://github.com/one-atom/one-atom/tree/react-fast-refersh-typescript

To Reproduce

Named export steps

1. Checkout said remote branch
2. Once node modules are installed, run the npm script build at root.
3. Go into $ cd modules/bundler/test/fixtures/run_dev_typescript
4. Run $ node run.dev.js
5. In port 8000, the error TestComponent is not defined is thrown

What we can do now it is to replace the named export to a default one. It'll stop throwing error but now if a change is made in react_fast_refresh_check.tsx it'll not recover state.

Filename recovering state steps

1. "fix" the Named export steps
2. Increment the counter
3. Make an update in react_fast_refresh_check.tsx
4. Counter is now reseted to 0.

Only when react_fast_refresh_check.tsx is renamed to TestComponent.tsx it'll not behave like this and actually recover the state.

Expected behavior

Should not need to do the changes above, it should work as the correspondent Babel setup.

Babel setup can be found here:
modules/bundler/test/fixtures/run_dev_typescript

It also as got an equivalent run.dev.js script

---

With that said, I would like to ask a question about ts transformers that's related to react-refresh but probably not for this package to handle.

At the company I work for we've been using React with TypeScript for a long time, way before official support was added, and up to this day we've been using a quite unconventional way to structure our components. They basically look like this:

// In a file components/example.component.ts
export namespace Example {
  export interface Props {
    title: string;
  }

  export const h: FC<Props> = function Example({ title }) {
    return <div>{title}</div>;
  }
}

// main.tsx
ReactDOM.render(<Example.h title="Hello World!" />, document.getElementById('root'));

while it's one of a kind, it solved issues we had getting TypeScript to fit into a large codebase without having types and lesser components bloating everything.

So my question is; do you know, with your knowledge, if it would be feasible to apply some transformation to get our unconventional components to utilize react-refresh as well?

From my understanding, react-refresh is quite picky to what it thinks is a React component. I figure since our components are always named .component., maybe a custom transformation can be done for these files to turn them into something react-refresh conciders as a recognizable component?

[Jack-Works]

Thanks for your report! I'll investigate your reproduction next week.

[Jack-Works]

if it would be feasible to apply some transformation to get our unconventional components to utilize react-refresh as well?

Yes, it is possible but not easy. Because TypeScript namespace has many special features, for example, you can contain both types and values in it, namespace can also have declaration merging. If you can figure out how many unusual patterns are there in your codebase, it might be easy to write an ad-hoc transformer.

react-refresh is quite picky to what it thinks is a React component

Transformers are used to "discover" the component, the runtime is okay with anything that comes in. When writing the TS version, I'm trying to match the behavior of the babel version

[Jack-Works]

In port 8000, the error TestComponent is not defined is thrown

Set module: "esnext" in tsconfig can resolve this problem. If you're bundling code by bundler, it would be better to offer ESModule to get better tree-shaking, etc.

[Jack-Works]

It'll stop throwing error but now if a change is made in react_fast_refresh_check.tsx it'll not recover state.

My test result shows that HMR will not work for the root React component, in this case, TestComponent. I created a new file App.tsx and export function App() { return <TestComponent /> }, then I use App as the root component that being passed into ReactDOM.render.

In this approach, HMR works now.

[EmilNordling]

Yes, it is possible but not easy. Because TypeScript ...

That good to know, thanks! Is there any good resources to learn this that you are recommending?

Set module: "esnext" in tsconfig can resolve this problem. ...
My test result shows that HMR will not work for the root React component ...

I mimicked the setup as you described and the issues I brought up are not present anymore, that's good! Tried it with both in the linked repo and a more real world application. Both of them, in the TypeScript only setup when adding a parent, works as intended, but there's now the minor difference that a Babel setup does work for the root React component. That might be a non-issue for most cases?

A thing that does not work with the adjusted setup is nullish coalescing for some reason? I've updated the branch that was linked: one-atom/one-atom@9607965 The error occurd in both the linked repo and a more real world application.

[Jack-Works]

That good to know, thanks! Is there any good resources to learn this that you are recommending?

Unfortunately, many TypeScript compiler APIs are undocumented and there is no clear doc to teach you how to write plugin/transformers. Google may help you to search for some articles.

Both of them, in the TypeScript only setup when adding a parent, works as intended, but there's now the minor difference that a Babel setup does work for the root React component. That might be a non-issue for most cases?

That's strange but I really didn't notice that. Yes, I believe typical use cases are not editing root components frequently (so I didn't notice that in our project). But I'll open an issue for it to investigate that later.

[Jack-Works]

"target": "ESNext",

module and target are two separate options. Your error in the picture is reported by Webpack. You need to set module to ESNext (and Webpack will handle it) but not target to ESNext (therefore TypeScript won't compile ?? syntax)

[EmilNordling]

Unfortunately, many TypeScript compiler APIs are undocumented ...

That's unfortunate, but it does sound like a nice thing to learn! Will have to check it out.

... but not target to ESNext (therefore TypeScript won't compile ?? syntax)

Oh I understand now, my bad! With this clarified I believe the issue can be closed now! Thank you :)