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

UpdateDefinitions type not exported from @reduxjs/toolkit/dist/query/endpointDefinitions #4492

Closed
joekrill opened this issue Jul 2, 2024 · 19 comments · Fixed by #4519
Closed

Comments

@joekrill
Copy link
Contributor

joekrill commented Jul 2, 2024

I'm trying to upgrade to Redux Toolkit v2 but running into a problem that I'm pretty sure is caused by the UpdateDefinitions type not being exported from @reduxjs/toolkit/query.

I have a monorepo with a separate package for my API endpoint, which is further split up into smaller chunks. @reduxjs/toolkit is a peer dependency in that package.

I have a base API defined like this:

export const baseApi = createApi({
  reducerPath: "api",
  endpoints: () => ({}),
});

And the additional API chunks are handled similar to this:

export const authApi = baseApi
  .enhanceEndpoints({
    addTagTypes: ["Permissions"],
  })
  .injectEndpoints({
    endpoints: (build) => ({
      permissions: build.query<PermissionsResponse, void>({
        query: () => "user/permissions",
        providesTags: () => [{ type: "Permissions", id: "CURRENT" }],
      }),
    }),
  });

This was working fine with v1, but after trying to upgrade to v2 I'm getting the error (at the authApi definition):

The inferred type of 'authApi' cannot be named without a reference to '@reduxjs/toolkit/dist/query/endpointDefinitions'. This is likely not portable. A type annotation is necessary.ts(2742)

If I remove the enhanceEndpoints call (and leave just the injectEndpoints call) the issues goes away, but then I can no longer use the "Permissions" tag in provideTags. I'm fairly certain I've narrowed down the problem to the fact that the UpdateDefinitions type used by enhanceEndpoints is not exported by @reduxjs/toolkit/query. All the other referenced types seem to be exported except for this one. Is there a specific reason for omitting it? Or was it an oversight? Would it be worth me putting up a PR to export it?

@markerikson
Copy link
Collaborator

We presumably never just thought to export it - there's been a lot of that going around :) (See #3962 , #4448 , etc.)

If exporting this helps, yeah, please file a PR!

@aryaemami59
Copy link
Contributor

aryaemami59 commented Jul 2, 2024

@joekrill Can you show what your tsconfig.json file looks like?

@joekrill
Copy link
Contributor Author

joekrill commented Jul 2, 2024

@aryaemami59 this is my tsconfig.json:

{

  "compilerOptions": {
    "noFallthroughCasesInSwitch": true,
    "noImplicitAny": true,
    "noUncheckedIndexedAccess": true,
    "strict": true,

    "module": "esnext",
    "moduleResolution": "bundler",
    "resolveJsonModule": true,
    "rootDir": "src",

    "declaration": true,
    "declarationMap": true,
    "outDir": "build",
    "sourceMap": true,

    "allowJs": true,

    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "isolatedModules": true,

    "jsx": "react-jsx",
    "lib": ["dom", "dom.iterable", "esnext"],
    "target": "es2020",

    "composite": true,

    "preserveWatchOutput": true,

    "skipLibCheck": true
  },
  "include": ["src/**/*"],
  "exclude": ["build/"]
}

@aryaemami59
Copy link
Contributor

aryaemami59 commented Jul 2, 2024

@joekrill

Can you run this command and try again to see if it fixes the issue:

npm install https://pkg.csb.dev/reduxjs/redux-toolkit/commit/06a30ee4/@reduxjs/toolkit

@joekrill
Copy link
Contributor Author

joekrill commented Jul 5, 2024

@aryaemami59 That build seems to have fixed the issue! Should I hold off on creating a PR to explicitly export the UpdateDefinitions type? Or is it still worth putting up a PR for that?

@aryaemami59
Copy link
Contributor

@joekrill That would be up to the maintainers but I would rather not export a type unless we absolutely have to.

@mchill
Copy link

mchill commented Jul 18, 2024

I'm seeing a similar problem with the lack of UpdateDefinitions export. My library that uses enhanceEndpoints builds fine, but when I import the enhanced API into another package, Typescript infers its type to be any.

A dumb workaround I have is to import and export UpdateDefinitions in my library like this:

import { UpdateDefinitions } from '@reduxjs/toolkit/dist/query/endpointDefinitions';
import { myApi as api } from './api';

export { UpdateDefinitions };

export const myApi = api.enhanceEndpoints({});

That fixes the type inference in the consuming package. Maybe I'm missing something simple in my tsconfig, but I haven't found a better solution yet.

Actually, this only suppresses the error, but types are still wrong. It looks like the problem happens when moduleResolution of the consuming package is set to bundler. Imports from @reduxjs/toolkit/dist/... don't work in that case.

@joekrill
Copy link
Contributor Author

@joekrill That would be up to the maintainers but I would rather not export a type unless we absolutely have to.

@aryaemami59 Well the reason I ask is because if the changes in the branch you asked me to try will eventually get merged, then that seems to fix the problem and there would be no need for me to create an additional PR for it. So I guess what I'm really asking is: do you expect the changes in that branch to make it into master?

@markerikson
Copy link
Collaborator

@joekrill yeah, Arya's been doing a lot of good work fixing up the various TS interop issues. I haven't had time yet to review any of them myself, but sounds like they're pretty close at this point.

@aryaemami59
Copy link
Contributor

@joekrill Yes. That PR is ready. It just needs to be reviewed and yes I expect it will get merged at some point.

@aryaemami59
Copy link
Contributor

@mchill The issue is you should not be importing anything from the dist folder.

@mchill
Copy link

mchill commented Jul 18, 2024

@mchill The issue is you should not be importing anything from the dist folder.

Totally agree, and I'm not directly, but the generated type of enhanceEndpoints looks like this:

import("@reduxjs/toolkit/query").Api<import("@reduxjs/toolkit/query").BaseQueryFn<string | FetchArgs, unknown, import("@reduxjs/toolkit/query").FetchBaseQueryError, {}, import("@reduxjs/toolkit/query").FetchBaseQueryMeta>, import("@reduxjs/toolkit/dist/query/endpointDefinitions").UpdateDefinitions<{}, never, never>, string, never, typeof import("@reduxjs/toolkit/query").coreModuleName | typeof import("@reduxjs/toolkit/dist/query/react").reactHooksModuleName>;

Both UpdateDefinitions and reactHooksModuleName are imported from dist/. Then when I publish those generated types to a package and import the package into another with "moduleResolution": "bundler", it obviously doesn't work.

Maybe there's something I should be doing differently to generate better types. I did find a real workaround by setting paths in the consumer package's tsconfig.json, but I'm aware of how bad practice it is.

{
    "compilerOptions": {
        "baseUrl": ".",
        "paths": {
            "@reduxjs/toolkit/dist/*": ["node_modules/@reduxjs/toolkit/dist/*"]
        }
    }
}

Haven't tried out your PR yet, but hopefully that fixes it.

@aryaemami59
Copy link
Contributor

@mchill Can you try out the PR and let me know if it fixes the issue you're having?

@mchill
Copy link

mchill commented Jul 18, 2024

It does! But it introduced another type error when adding dynamicMiddleware to my store.

Exported variable 'store' has or is using name 'DynamicDispatch' from external module "/path/to/project/node_modules/@reduxjs/toolkit/dist/index" but cannot be named.

@aryaemami59
Copy link
Contributor

@mchill Can you share the code snippet? And are you sure you're not importing from the dist folder?

@mchill
Copy link

mchill commented Jul 19, 2024

Sure, here's a very stripped down example. Definitely no dist imports.
https://github.com/mchill/rtk-demo

@aryaemami59
Copy link
Contributor

@mchill Thanks for the repro, I fixed the issue, can you try this and let me know if it fixes the problem?

npm install https://pkg.csb.dev/reduxjs/redux-toolkit/commit/51afa9fc/@reduxjs/toolkit

@mchill
Copy link

mchill commented Jul 22, 2024

Everything looks good to me with that version.

@markerikson
Copy link
Collaborator

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 a pull request may close this issue.

4 participants