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

Import react with "* as React" to prevent the need to use allowSyntheticDefaultExports/esModuleInterop in consumers (issue #110) #112

Merged
merged 1 commit into from
Aug 10, 2019

Conversation

ethanae
Copy link
Contributor

@ethanae ethanae commented Aug 9, 2019

Consumers of the lib are forced to add either allowSynthenticDefaultExports or esModuleInterop. While either of these may be common-place, it does allow clients to use their preferred import syntax and tsconfig.

This PR removes the synthetic default import syntax only from the publicly exposed modules of the library.

This fixes #110.

- synthetic default imports to es6 imports
@kevinsqi kevinsqi changed the title - targets issue #110 Import react with "* as React" to prevent the need to use allowSyntheticDefaultExports/esModuleInterp in consumers (issue #110) Aug 10, 2019
@kevinsqi kevinsqi changed the title Import react with "* as React" to prevent the need to use allowSyntheticDefaultExports/esModuleInterp in consumers (issue #110) Import react with "* as React" to prevent the need to use allowSyntheticDefaultExports/esModuleInterop in consumers (issue #110) Aug 10, 2019
@kevinsqi
Copy link
Owner

I confirmed that this fixes the issue against a test repo: https://github.com/kevinsqi/react-circular-progressbar-issue-110

@ethanae thank you for the contribution!

@kevinsqi kevinsqi merged commit 5b4898a into kevinsqi:master Aug 10, 2019
@ethanae
Copy link
Contributor Author

ethanae commented Aug 11, 2019

Great news! Keep up the good work @kevinsqi!

@Rodrico
Copy link

Rodrico commented Oct 20, 2019

Thanks for work on this item. Unfortunately it seems I met different kind of similar issue.

  • Using version ^2.0.2 along with some other packages.
  • Can't set allowSynthenticDefaultExports=true as it seems to be not compatible with other packages like react-numeric-input
  • only place where our strict TS build fails is import line in react-circular-progressbar/dist/types.d.ts where I'd need to change
    import React from 'react';
    to
    import * as React from 'react';

for development testing I just changed that line in my local /node_modules/ and it works just great but for production I had to remove react-circular-progressbar for now because of that.

Do you think that's something we can change here or is it something which would break other users of this awesome package?

I remmember I used this stack of dependencies in my TS build some time ago in different project just with older versions of these packages so I'm still suspecting myself I can have bug somewhere...

Thanks in advance :)

@ethanae
Copy link
Contributor Author

ethanae commented Oct 21, 2019

@Rodrico, just by looking at the source again it looks like you're correct. I can't figure out how I missed this with my testing of my changes. I'll do some digging later today.

@kevinsqi, this may need to be re-opened.

@kevinsqi
Copy link
Owner

@Rodrico could you throw up a quick repo (e.g. with create-react-app --typescript or Codesandbox) that reproduces the issue you're seeing, just so I can make sure whatever fix we come up with actually solves the problem? Thank you!

@Rodrico
Copy link

Rodrico commented Oct 25, 2019

Sorry for delay.
I created https://github.com/Rodrico/reactdemo/tree/master/ReactDemo
hope it's gonna be available to you, it's my first repo here.
Steps to reproduce in commit message.

Also, that repo is pretty heavyweight, requiring updated Visual Studio 2019 - it most probably could be reproduced in much more minimalized scenario - I used skeleton from my current project so it's as close as possible to real scenario.
Thanks :)

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

Successfully merging this pull request may close these issues.

Forced allowSyntheticDefaultImports in tsconfig
3 participants