Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Components inside typescript namespaces cause ReferenceError #22413

Closed
Enteleform opened this issue Sep 24, 2021 · 9 comments · Fixed by #22621
Closed

Bug: Components inside typescript namespaces cause ReferenceError #22413

Enteleform opened this issue Sep 24, 2021 · 9 comments · Fixed by #22621

Comments

@Enteleform
Copy link

Forwarded from vitejs/vite#3900 by @not-rusty based on the recommendation of @sodatea, which stated that this should be considered as a bug in the react-refresh package.
 


Describe the bug

Apparently components inside namespaces is not supported. I don't exactly know if the error comes from esbuild or something, but it would nice.

The bug is that is not shown as a compilation error.

Reproduction

Simply start a react-ts project with $ npm init @vitejs/app my-vue-app --template react-ts and write the following code:

namespace Lol {
  export const Lol = () ={
    return (
      <div>
        <p>Some component</p>
      </div>
    );
  };
}

function App() {
  return (
    <div className="App">
      <header className="App-header">
        <Lol.Lol />
        <p>Hello Vite + React!</p>
      </header>
    </div>
  );
}

export default App;

System Info

  System:
    OS: Linux 5.8 Ubuntu 20.04.2 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Core(TM) i5-10210U CPU @ 1.60GHz
    Memory: 440.54 MB / 7.47 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 14.16.0 - /usr/local/bin/node
    npm: 6.14.13 - /usr/local/bin/npm
  Browsers:
    Chrome: 90.0.4430.93
    Firefox: 89.0.1

Used package manager: npm

Logs

I get the following runtime error:

App.tsx:13 Uncaught ReferenceError: _c is not defined
    at App.tsx:13
    at App.tsx:14

I suppose it should be a compilation error?

@Enteleform Enteleform added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Sep 24, 2021
@irinakk
Copy link
Contributor

irinakk commented Sep 26, 2021

To add more details, namespace with a capitalized function member is transformed into this piece of code:

var N;
(function(N2) {
  N2.Test = () => {
  };
  _c = N2.Test; // Uncaught ReferenceError: _c is not defined
})(N || (N = {}));

The error occurs in vite's development mode since the application code is imported via <script type="module">, which creates strict mode.

@bvaughn
Copy link
Contributor

bvaughn commented Sep 27, 2021

cc @gaearon To confirm if this is a known/expected limitation.

@todd-elvers
Copy link

I have also encountered this error using Vite & react-refresh on a TypeScript react app that has react components nested under namespaces

@Dodd2013

This comment has been minimized.

@gaearon gaearon added Difficulty: medium good first issue Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Oct 14, 2021
@gaearon
Copy link
Collaborator

gaearon commented Oct 14, 2021

To confirm if this is a known/expected limitation.

No, it's not expected. This seems like a bug in the transform.

Transform: https://github.com/facebook/react/blob/main/packages/react-refresh/src/ReactFreshBabelPlugin.js

Tests:

If somebody wants to take this one, the first step is to produce a failing integration test demonstrating the issue. (Using compiled TS output as the "source" is fine.)

@owen-kosman
Copy link

@gaearon I see that no one has started work on this issue, I'll have a crack at this.

@ideffix
Copy link

ideffix commented Oct 20, 2021

@owen-kosman @gaearon I digged in a little bit into this issue and it looks like it only happends when code with namespace keyword is being passed to react-refresh/babel so I think it's not possible to reproduce it using just JS (so it's hard to write unit/integration tests). Check code where I present difference between TS compiled code and code that includes namespace.

I manage to fix it but I haven't add any tests because of issue mentioned above. If it's fine here is PR #22601

@irinakk
Copy link
Contributor

irinakk commented Oct 21, 2021

@ideffix Hi I think your fix does not solve the real problem here. I digged a little more and found that in this specific babel config in this issue.

const babelOpts = {
  parserOpts: {
    plugins: ["jsx", "typescript"]
  },
  plugins: [["react-refresh/babel.js"]]
};

this line of code gives a false assumption.

  case 'ExportNamedDeclaration':
    insertAfterPath = path.parentPath;
    programPath = insertAfterPath.parentPath; // => error here, this one is not Program
    break;

It seems babel parses namespace syntax into a structure with Program ->TSModuleDeclaration -> TSModuleBlock -> ExportNamedDeclaration, which in normal function exports we would expect Program -> ExportNamedDeclaration.

So I think the fix should be related, i'm not familiar with babel parser and not sure the best way to fix this, but hope the info helps.

@gaearon
Copy link
Collaborator

gaearon commented Nov 9, 2021

Should be fixed in react-refresh@0.11.0. Please ask Vite to upgrade and see if that fixes it.

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

Successfully merging a pull request may close this issue.

9 participants