-
-
Notifications
You must be signed in to change notification settings - Fork 253
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(utils): improve devtools typing to avoid depending on @redux-devtools/extension #777
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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 3dc8f03:
|
It might be a better solution to move |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, it's basically doing the same as pmndrs/zustand#1207 ?
It looks okay then.
src/vanilla/utils/devtools.ts
Outdated
type EnhancerOptions = Parameters< | ||
NonNullable<(typeof window)['__REDUX_DEVTOOLS_EXTENSION__']>['connect'] | ||
>[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though, this looks better. Curious how this is failing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. We still need to install the package.
I wonder if we can type check on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why L124 doesn't cause a type error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because it disappears, okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. that's right. I also confirmed 208d3c9 fixes #776. thanks for updating my fix! But it looks #713 feature does not work with TypeScript.. (tsc fails when we add some parameters that is coming from const state = proxy({ count: 0, text: 'hello' });
devtools(state, { name: 'state name', enabled: true, latency: 1000 }); // 'latency' is coming from `@redux-devtools/extensions` Overload 1 of 2, '(proxyObject: { count: number; text: string; }, options?: { enabled?: boolean | undefined; name?: string | undefined; } | undefined): (() => void) | undefined', gave the following error.
Argument of type '{ name: string; enabled: true; latency: number; }' is not assignable to parameter of type '{ enabled?: boolean | undefined; name?: string | undefined; }'.
Object literal may only specify known properties, and 'latency' does not exist in type '{ enabled?: boolean | undefined; name?: string | undefined; }'.
Overload 2 of 2, '(proxyObject: { count: number; text: string; }, name?: string | undefined): (() => void) | undefined', gave the following error.
Argument of type '{ name: string; enabled: boolean; latency: number; }' is not assignable to parameter of type 'string'.
15 devtools(state, { name: 'state name', enabled: true, latency: 1000 }); |
It seems if we install const state = proxy({ count: 0, text: 'hello' });
devtools(state, { name: 'state name', enabled: true, latency: 1000 }); // 'latency' is coming from `@redux-devtools/extensions` tsconfig.json {
"include": [
"./node_modules/@redux-devtools/extension/lib/types/index.d.ts",
]
} |
Hmm, isn't it just enough to install import type {} from '@redux-devtools/extension'
import { devtools } from 'valtio/utlis' |
Related Issues or Discussions
Fixes #776
Summary
I found the same issue in zustand(pmndrs/zustand#1205 (comment)), so I added the same workaround(copied the type defs from
@redux-devtools/extension
)I'm not sure if this way is a desired way, but I did it anyway..
Check List
yarn run prettier
for formatting code and docs