Skip to content

refactor(vanilla): resolve todo by removing type assertion using inferred type predicates #3134

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

Conversation

2yunseong
Copy link
Contributor

Related Bug Reports or Discussions

Fixes #

Summary

Refer to the TypeScript issue 37663: T | (() => T). (you can see the issue in the previous TODO comment too)

Before TypeScript 5.5, type inference with typeof was not possible in this case.

However, in TS 5.5+, this is now supported thanks to Inferred Type Predicates.

image

And I used it to resolve the todo.

Check List

  • pnpm run fix for formatting and linting code and docs

Copy link

vercel bot commented May 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
zustand-demo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2025 7:42am

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.

Copy link

pkg-pr-new bot commented May 23, 2025

demostarter

npm i https://pkg.pr.new/zustand@3134

commit: 31872b0

const createStoreImpl: CreateStoreImpl = (createState) => {
type TState = ReturnType<typeof createState>
type Listener = (state: TState, prevState: TState) => void
let state: TState
const listeners: Set<Listener> = new Set()

const setState: StoreApi<TState>['setState'] = (partial, replace) => {
// TODO: Remove type assertion once https://github.com/microsoft/TypeScript/issues/37663 is resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This issue isn't closed.

typeof partial === 'function'
? (partial as (state: TState) => TState)(state)
: partial
const nextState = isFunction(partial) ? partial(state) : partial
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the goal.
It should be this:

Suggested change
const nextState = isFunction(partial) ? partial(state) : partial
const nextState = typeof partial === 'function' ? partial(state) : partial

@dai-shi
Copy link
Member

dai-shi commented May 23, 2025

Thanks. Let's keep eyes on the upstream issue.

@dai-shi dai-shi closed this May 23, 2025
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.

2 participants