Skip to content

add TS 5.5 to test matrix #4500

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

Closed
wants to merge 2 commits into from
Closed

add TS 5.5 to test matrix #4500

wants to merge 2 commits into from

Conversation

EskiMojo14
Copy link
Collaborator

No description provided.

Copy link

codesandbox bot commented Jul 8, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

Copy link

netlify bot commented Jul 8, 2024

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

Name Link
🔨 Latest commit 701b9ef
🔍 Latest deploy log https://app.netlify.com/sites/redux-starter-kit-docs/deploys/6696dde908ebe80008418019
😎 Deploy Preview https://deploy-preview-4500--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.

Copy link

codesandbox-ci bot commented Jul 8, 2024

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 701b9ef:

Sandbox Source
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration
rtk-esm-cra Configuration

Copy link

github-actions bot commented Jul 8, 2024

size-limit report 📦

Path Size

@aryaemami59
Copy link
Member

const counterReducer1: Reducer<number> = () => 0
const counterReducer2: Reducer<number> = () => 0

- const counterReducer1: Reducer<number> = () => 0
- const counterReducer2: Reducer<number> = () => 0
+ const counterReducer1 = () => 0
+ const counterReducer2 = () => 0

This change seems to fix the issue related to TS 5.5.

@EskiMojo14
Copy link
Collaborator Author

const counterReducer1: Reducer<number> = () => 0
const counterReducer2: Reducer<number> = () => 0

- const counterReducer1: Reducer<number> = () => 0
- const counterReducer2: Reducer<number> = () => 0
+ const counterReducer1 = () => 0
+ const counterReducer2 = () => 0

This change seems to fix the issue related to TS 5.5.

that seems... bad?
it looks like the reverse mapping used by ReducersMapObject seems to have broken somehow

@aryaemami59
Copy link
Member

it looks like the reverse mapping used by ReducersMapObject seems to have broken somehow

Yeah I'm not really sure why tbh, this also fixes it:

const counterReducer1: Reducer<number> = () => 0
const counterReducer2: Reducer<number> = () => 0

- const counterReducer1: Reducer<number> = () => 0
- const counterReducer2: Reducer<number> = () => 0
+ const counterReducer1: Reducer<number, UnknownAction> = () => 0
+ const counterReducer2: Reducer<number, UnknownAction> = () => 0

@EskiMojo14
Copy link
Collaborator Author

that makes no sense, UnknownAction is the default for that type parameter anyway 🤔
Reducer<number> and Reducer<number, UnknownAction> should be equivalent.

wtf typescript? 😅

@aryaemami59
Copy link
Member

aryaemami59 commented Jul 9, 2024

that makes no sense, UnknownAction is the default for that type parameter anyway 🤔
Reducer<number> and Reducer<number, UnknownAction> should be equivalent.

Yeah I was thinking the exact same thing, I tried to dig to see what exactly causes the issue but I kind of gave up. I think it might be because when we explicitly pass in generic type parameters to Reducer, the type for preloadedState does not get inferred. It just becomes the default State because TS can't do partial inference for generic type parameters. I could be very well wrong though.

Edit: My previous hunch is not correct, You're right. It has something to do with ReducersMapObject.

@EskiMojo14
Copy link
Collaborator Author

@aryaemami59 are you able to recreate it in the TS Playground? weirdly i can't seem to reproduce it there: https://tsplay.dev/N7kyEN

@Andarist
Copy link

Andarist commented Jul 9, 2024

TS playground repro: link, the change bisects to microsoft/TypeScript#57837

@Andarist
Copy link

Andarist commented Jul 9, 2024

Actual minimal repro: TS playground

Consider me nerd-sniped. I will be digging into it more tomorrow.

@aryaemami59
Copy link
Member

This looks very interesting.

@Andarist
Copy link

microsoft/TypeScript#59232

@markerikson
Copy link
Collaborator

Where do we stand on this one?

@markerikson
Copy link
Collaborator

I think this is now a dupe of #4567 .

@EskiMojo14 EskiMojo14 deleted the ts5.5 branch September 5, 2024 14:28
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.

4 participants