Skip to content

add TS 5.5 to test matrix#4500

Closed
EskiMojo14 wants to merge 2 commits intomasterfrom
ts5.5
Closed

add TS 5.5 to test matrix#4500
EskiMojo14 wants to merge 2 commits intomasterfrom
ts5.5

Conversation

@EskiMojo14
Copy link
Copy Markdown
Collaborator

No description provided.

@codesandbox
Copy link
Copy Markdown

codesandbox bot commented Jul 8, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@netlify
Copy link
Copy Markdown

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.

@codesandbox-ci
Copy link
Copy Markdown

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

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 8, 2024

size-limit report 📦

Path Size

@aryaemami59
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown

Andarist commented Jul 9, 2024

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

@Andarist
Copy link
Copy Markdown

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
Copy Markdown
Member

This looks very interesting.

@Andarist
Copy link
Copy Markdown

microsoft/TypeScript#59232

@markerikson
Copy link
Copy Markdown
Collaborator

Where do we stand on this one?

@markerikson
Copy link
Copy Markdown
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