reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.63k stars 1.15k forks source link

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

Closed joekrill closed 1 month ago

joekrill commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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

joekrill commented 2 months ago

@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 commented 2 months ago

@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 commented 2 months ago

@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 commented 1 month ago

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

mchill commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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

aryaemami59 commented 1 month ago

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

mchill commented 1 month ago

@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 commented 1 month ago

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

mchill commented 1 month ago

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 commented 1 month ago

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

mchill commented 1 month ago

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

aryaemami59 commented 1 month ago

@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 commented 1 month ago

Everything looks good to me with that version.

markerikson commented 1 month ago

Fixed in https://github.com/reduxjs/redux-toolkit/releases/tag/v2.2.7 !