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

fix(RTKQuery): Resolves type portability with createApi. fixes #3568 #3569

Closed

Conversation

eric-crowell
Copy link
Contributor

@eric-crowell eric-crowell commented Jun 29, 2023

Exports additional types to resolve the ts2742 error when using the createApi function from RTK Query.

Resolves issue: #3568

@codesandbox
Copy link

codesandbox bot commented Jun 29, 2023

This branch is running in CodeSandbox. Use the links below to review this PR faster.


CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders | Preview

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 29, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8c985cd:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration
rtk-esm-cra Configuration

@netlify
Copy link

netlify bot commented Jun 29, 2023

Deploy Preview for redux-starter-kit-docs ready!

Name Link
🔨 Latest commit 8c985cd
🔍 Latest deploy log https://app.netlify.com/sites/redux-starter-kit-docs/deploys/64aea119bbd3dd0008b95440
😎 Deploy Preview https://deploy-preview-3569--redux-starter-kit-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

export { defaultSerializeQueryArgs } from './defaultSerializeQueryArgs'
export * from './tsHelpers';
Copy link
Member

Choose a reason for hiding this comment

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

Please try to keep it to a minimum and check what is actually needed. From what I gather, that might actually only be symbol types.
We don't want to export too much, since at that point implementation details become a public API that we have to try to keep stable and keep supporting pretty much indefinitely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense; you got it! I'll pin point it.

Maybe you're right about the symbols and only coreModuleName needs to be exposed. TypeScript has a hard time making it clear.

Here's a very simple reproduction of all the portability errors I was seeing in my project:
https://codesandbox.io/p/sandbox/rtk-type-portability-j99lyh?file=%2Fsrc%2Frtk%2FmyApiBaseQuery.ts%3A11%2C3

^ Additional comments in the sandbox to see what might've lead me astray.

@eric-crowell eric-crowell force-pushed the module_symbol_export branch from a697971 to 33268d9 Compare July 6, 2023 10:06
@eric-crowell
Copy link
Contributor Author

You are correct @phryneas, it is just the symbol type that is necessary to resolve this issue.

@eric-crowell eric-crowell force-pushed the module_symbol_export branch from 33268d9 to 8f02cd8 Compare July 6, 2023 17:49
@eric-crowell eric-crowell requested a review from phryneas July 6, 2023 17:49
@phryneas
Copy link
Member

phryneas commented Jul 6, 2023

While you're at it, could you make sure to export the reactHooksModuleName from query/react in the same way?

…n exporting declarations in a typescript project
@eric-crowell eric-crowell force-pushed the module_symbol_export branch from 8f02cd8 to 3a1b28d Compare July 6, 2023 18:46
@eric-crowell
Copy link
Contributor Author

While you're at it, could you make sure to export the reactHooksModuleName from query/react in the same way?

Sure thing!

@eric-crowell eric-crowell force-pushed the module_symbol_export branch from 7304993 to 8c985cd Compare July 12, 2023 12:48
@eric-crowell
Copy link
Contributor Author

I found additional problems inferring types when using configureStore. This is only when API reducers (from createApi) are included.

Reproduction of the error on Code Sandbox

Exporting the exact types QueryCacheKey, RootState, CombinedState from ./core/apiState resolves the issue.

@markerikson
Copy link
Collaborator

This PR had conflicts and I didn't have push permissions, so I duped and rebased over into #3678 . 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.

3 participants