-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
chore: Fix TS lang server support in our .d.ts
files
#4698
Conversation
@@ -30,14 +32,14 @@ export interface ComponentHooks { | |||
_pendingEffects: EffectHookState[]; | |||
} | |||
|
|||
export interface Component extends PreactComponent<any, any> { | |||
export interface Component extends Omit<PreactComponent<any, any>, '_renderCallbacks'> { |
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.
This PR adds a healthy sprinkling of Omit
which I think is correct given our use cases -- generally, our internal types intentionally clash with the base/user-exposed types.
interface BaseHookState { | ||
_value?: unknown; | ||
_nextValue?: undefined; | ||
_pendingValue?: undefined; | ||
_args?: undefined; | ||
_pendingArgs?: undefined; | ||
_component?: undefined; | ||
_cleanup?: undefined; | ||
_nextValue?: unknown; | ||
_pendingValue?: unknown; | ||
_args?: unknown; | ||
_pendingArgs?: unknown; | ||
_component?: unknown; | ||
_cleanup?: unknown; | ||
} |
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.
undefined
is a more specific type than unknown
which disallows some interfaces from extending these properties as they need to.
"compat/**/*.d.ts", | ||
"jsx-runtime/**/*.d.ts" |
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.
compat & jsx-runtime source & test files throw up some errors yet but I think it's worth linting their types at least as these are user-exposed. Will help guard against typos and what not
fn: ForwardFn<P, R> | ||
fn: ForwardRefRenderFunction<R, P> |
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.
This is from #4675 -- ForwardFn
is not the name in React & notably it has the generics reversed. It was marked as deprecated in that PR so all this is doing is replacing it with the correct, non-deprecated version (the internals of both types match).
Long been a bit of a headache, we don't get an TS support in our
.d.ts
files which allows typos, usused values, etc. to run a bit rampant. Turns out it's a simple issue:jsconfig.json
hasskipLibCheck
set tofalse
by default, and this is seemingly undocumented.Easy fix and has surfaced a few (internal) errors already.