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

Prefer import * as for imports whose types are re-exported #7742

Conversation

devrelm
Copy link
Contributor

@devrelm devrelm commented Feb 20, 2021

Fixes #7741

Using import React from 'react' causes issues when the import makes
its way into the d.ts file. Users without esModuleInterop: true or
allowSyntheticDefaultImports: true (or skipLibCheck: true) in
their tsconfig files will get errors from typescript about not being
able to default-import using that syntax.

This PR fixes the specific imports that would cause issues due to
their types being re-exported in @apollo/client's types.

Checklist:

  • N/A If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
  • N/A Make sure all of the significant new logic is covered by tests

@apollo-cla
Copy link

@devrelm: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

@devrelm
Copy link
Contributor Author

devrelm commented Feb 20, 2021

Well this clearly isn't working. There's now a bunch of errors that I didn't expect to come up. I'll see what I can do, but this seems like a bit of a catch-22 situation.

@devrelm devrelm force-pushed the devrelm/no-synthetic-default-imports branch 2 times, most recently from 78588fd to 82ca81b Compare February 21, 2021 02:35
@devrelm
Copy link
Contributor Author

devrelm commented Feb 21, 2021

I think I have everything working and it was simpler than I'd thought.

The errors that I ran into with f3fe053 seem to have stemmed from zen-observable not having good ESM support. I even tried importing from zen-observable/esm, though that wasn't working either.

It appears that that's a known issue, and that the apollographql org even publishes a zen-observable-ts package to deal with it. So I just updated the dependency and imported that instead.

@benjamn
Copy link
Member

benjamn commented Feb 22, 2021

@devrelm Can you rebase this PR against the release-3.4 branch, and update the target branch to release-3.4? The zen-observable-ts changes have already happened on that branch (see #7615).

@devrelm devrelm force-pushed the devrelm/no-synthetic-default-imports branch from 82ca81b to 62ad4c5 Compare February 23, 2021 02:16
@devrelm devrelm changed the base branch from main to release-3.4 February 23, 2021 02:16
@devrelm devrelm force-pushed the devrelm/no-synthetic-default-imports branch from 62ad4c5 to 4efaa9f Compare February 23, 2021 02:17
@devrelm
Copy link
Contributor Author

devrelm commented Feb 23, 2021

@benjamn Done. Thanks!

@benjamn benjamn added this to the Release 3.4 milestone Feb 23, 2021
Fixes apollographql#7741

Using `import React from 'react'` causes issues when the import makes
its way into the d.ts file. Users without `esModuleInterop: true` or
`allowSyntheticDefaultImports: true` (or `skipLibCheck: true`) in
their tsconfig files will get errors from typescript about not being
able to default-import using that syntax.

This PR fixes the specific imports that would cause issues due to
their types being re-exported in `@apollo/client`'s types.
@benjamn benjamn force-pushed the devrelm/no-synthetic-default-imports branch from 4efaa9f to 2709331 Compare March 5, 2021 17:39
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your patience @devrelm (and thanks in advance for testing these changes in the next v3.4 beta release 😉)!

@benjamn benjamn merged commit 9fb964e into apollographql:release-3.4 Mar 5, 2021
@benjamn benjamn removed the 🏓 awaiting-contributor-response requires input from a contributor label Mar 5, 2021
benjamn added a commit that referenced this pull request Mar 9, 2021
@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants