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: Add types to ensure that there will be no compilation errors when struct: false #1851

Merged

Conversation

duan602728596
Copy link
Contributor

@duan602728596 duan602728596 commented Dec 16, 2021

I used the v8 version of react-redux in the project.

When typescript is configured with strict: true, The type of let listeners = [] is recognized as any[].

When typescript is configured with strict: false, The type of let listeners = [] is recognized as never[],
This will cause listeners.push(listener) to prompt the error TS2345: Argument of type 'Listener' is not assignable to parameter of type'never'..

When project is not configured with strict: true, this will cause a project compile-time error, so I added a type here to solve this problem.

@codesandbox-ci
Copy link

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 4ed2047:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@netlify
Copy link

netlify bot commented Dec 16, 2021

✔️ Deploy Preview for react-redux-docs ready!

🔨 Explore the source changes: 4ed2047

🔍 Inspect the deploy log: https://app.netlify.com/sites/react-redux-docs/deploys/61bb0df5707f2f0007a95c7a

😎 Browse the preview: https://deploy-preview-1851--react-redux-docs.netlify.app

@timdorr
Copy link
Member

timdorr commented Dec 16, 2021

Thanks!

@timdorr timdorr merged commit 8622b18 into reduxjs:master Dec 16, 2021
@markerikson
Copy link
Contributor

Huh. Yeah, the added type for the array is useful here, but I'm also curious how you even ran into this. This is an internal structure that isn't exposed to consumers of the library. Did you have your app configured to try to compile dependencies out of node_modules or something?

@duan602728596
Copy link
Contributor Author

Huh. Yeah, the added type for the array is useful here, but I'm also curious how you even ran into this. This is an internal structure that isn't exposed to consumers of the library. Did you have your app configured to try to compile dependencies out of node_modules or something?

Maybe I didn't ignore node_modules, so I ran into this problem.
This is a demo that I can reproduce: https://github.com/duan602728596/react-redux-build-project.
In this demo, the files in node_modules/react-redux/src were checked during compilation and an error was given.
This is the case with my project.

@markerikson
Copy link
Contributor

Yeah, I would normally advise against trying to compile or type-check any code in node_modules :)

@duan602728596
Copy link
Contributor Author

Yeah, I would normally advise against trying to compile or type-check any code in node_modules :)

Yes, I will pay attention to these in the future.

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