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 all uses of /dist/ in type imports #3516

Closed
markerikson opened this issue Jun 9, 2023 · 5 comments
Closed

Fix all uses of /dist/ in type imports #3516

markerikson opened this issue Jun 9, 2023 · 5 comments
Milestone

Comments

@markerikson
Copy link
Collaborator

Per Andrew Branch, these are bad and breaking things:

import type { Api } from '@reduxjs/toolkit/dist/query/apiTypes'

import type {
  QuerySubState,
  SubscriptionOptions,
  QueryKeys,
  RootState,
} from '@reduxjs/toolkit/dist/query/core/apiState'

// etc

They should just import from the packages instead, like @reduxjs/toolkit/query.

@markerikson markerikson added this to the 2.0 milestone Jun 9, 2023
@eric-crowell
Copy link
Contributor

Would it be preferred to use relative pathing to these types instead of from the package's main export?
Otherwise, all those types would also have to be publicly exported and supported? (I think, maybe I'm wrong)

Wanted to ask since I was already patching a lot of this to fix my projects using 2.0. Hoping I can put out a PR for this issue.

@phryneas
Copy link
Member

@eric-crowell That would lead the packages to merge into each other at bundling, which you definitely don't want to happen.

@eric-crowell
Copy link
Contributor

@eric-crowell That would lead the packages to merge into each other at bundling, which you definitely don't want to happen.

Ah, okay. I agree, I definitely don't want that to happen. Will keep with the suggested fix if I can make a PR.

@eric-crowell
Copy link
Contributor

eric-crowell commented Jul 18, 2023

@phryneas As I'm working this, I'm not sure how to import these without relative pathing or exposing them publicly.

Screen Shot 2023-07-18 at 11 33 10 AM

With ESM, @reduxjs/toolkit looks like a single package with different entry points now. So maybe bundling together isn't bad? I know Vite is pretty smart with bundling shared resources per entry point (with relative imports). For type imports, I don't think they'll be any bundling. Just need to make sure there's nothing circular.

How should I approach?

@markerikson
Copy link
Collaborator Author

Done in #3672 .

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

No branches or pull requests

3 participants