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

Forced allowSyntheticDefaultImports in tsconfig #110

Closed
ethanae opened this issue Aug 3, 2019 · 10 comments · Fixed by #112
Closed

Forced allowSyntheticDefaultImports in tsconfig #110

ethanae opened this issue Aug 3, 2019 · 10 comments · Fixed by #112

Comments

@ethanae
Copy link
Contributor

ethanae commented Aug 3, 2019

I just added this to my project. Very neat. However, is it necessary to force client tsconfigs to enable allowSyntheticDefaultImports? Looking at the react-circular-progressbar types there are only two imports to React using synthetic default imports.

@kevinsqi
Copy link
Owner

kevinsqi commented Aug 7, 2019

Interesting. So you're saying that the import React from 'react' statements in this library cause downstream projects to need to set allowSyntheticDefaultImports: true when they normally would not?

Would this be fixed by using import * as React from 'react'? If so, I'd be willing to make that change. Would you be able to create a create-react-app-typescript CodeSandbox that demonstrates the issue? That'd help me make sure that a fix actually resolves the issue. Thank you!

@ethanae
Copy link
Contributor Author

ethanae commented Aug 7, 2019

Interesting. So you're saying that the import React from 'react' statements in this library cause downstream projects to need to set allowSyntheticDefaultImports: true when they normally would not?

I also thought downstream projects wouldn't be affected. I cannot say for sure whether this is solely tsc or a combination of tsc and possibly react types. I'll try find another lib also with allowSyntheticDefaultImports: true to test it out. For now, maybe you can make more sense of it?

Here's a CodeSandbox.

I originally thought it might have been my tsc version, 3.5, but from #942 it seems like CodeSandbox is fixed to 3.3.3 and sees the same type error.

@ethanae
Copy link
Contributor Author

ethanae commented Aug 7, 2019

import * as React from 'react' would indeed fix the issue. I wouldn't mind making a PR should you decide this is the way to go.

@kevinsqi
Copy link
Owner

kevinsqi commented Aug 7, 2019

A PR would be great! Thanks for looking into this.

ethanae pushed a commit to ethanae/react-circular-progressbar that referenced this issue Aug 7, 2019
- synthetic default exports to es6 exports
ethanae pushed a commit to ethanae/react-circular-progressbar that referenced this issue Aug 9, 2019
- synthetic default imports to es6 imports
@kevinsqi
Copy link
Owner

I was able to reproduce the issue in this repo: https://github.com/kevinsqi/react-circular-progressbar-issue-110

For posterity, this is the exact tsc error I got when allowSyntheticDefaultImports=false:

Failed to compile.

/Users/kevin/devel/rcp-issue-110/src/App.tsx
TypeScript error in /Users/kevin/devel/rcp-issue-110/src/App.tsx(10,7):
JSX element type 'CircularProgressbar' is not a constructor function for JSX elements.
  Type 'CircularProgressbar' is missing the following properties from type 'ElementClass': context, setState, forceUpdate, props, and 2 more.  TS2605

     8 |   return (
     9 |     <div className="App">
  > 10 |       <CircularProgressbar value={50} />
       |       ^
    11 |     </div>
    12 |   );
    13 | }

kevinsqi added a commit that referenced this issue Aug 10, 2019
Import react with "* as React" to prevent the need to use allowSyntheticDefaultExports/esModuleInterop in consumers (issue #110)
@kevinsqi
Copy link
Owner

@ethanae merged your PR and released it in v2.0.2. I've tested it out and it appears to work - please let me know if you still encounter any issues. Thank you for raising the issue and submitting a fix!

@Rodrico
Copy link

Rodrico commented Oct 20, 2019

I was not sure where to continue and I added my findings to #112 hope it works as well :)
Thanks.

@kevinsqi kevinsqi reopened this Oct 22, 2019
@kevinsqi
Copy link
Owner

kevinsqi commented Oct 26, 2019

@Rodrico I realized there were two other import React from 'React' references in the code, which was the cause of the error you were seeing. I fixed those references in #124 and published that in react-circular-progressbar 2.0.3. Can you try v2.0.3 and check if that fixes the problem?

@kevinsqi
Copy link
Owner

@Rodrico does v2.0.3 work for you now? If so, I can close this issue out :)

@kevinsqi
Copy link
Owner

@Rodrico I haven't heard back in a while so I'm going to close this issue. But if the issue you had persists in v2.0.3, please let me know!

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