-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 up compat types and tests #1740
Conversation
@@ -179,7 +178,6 @@ function normalizeVNode(vnode) { | |||
function cloneElement(element) { | |||
if (!isValidElement(element)) return element; | |||
let vnode = normalizeVNode(preactCloneElement.apply(null, arguments)); | |||
vnode.$$typeof = REACT_ELEMENT_TYPE; |
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 already set in options.vnode
hook which preactCloneElement
calls so no need to reset here I think
compat/src/index.d.ts
Outdated
@@ -53,11 +53,11 @@ declare namespace React { | |||
|
|||
export function unmountComponentAtNode(container: Element | Document | ShadowRoot | DocumentFragment): boolean; | |||
|
|||
export function createFactory(type: preact.VNode["type"]): preact.VNode<{}>; | |||
export function createFactory(type: preact.VNode["type"]): (props?: any, ...children: preact.ComponentChildren[]) => preact.VNode<any>; |
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.
export function isValidElement(element: any): boolean; | ||
export function findDOMNode(component: preact.Component): Element | null; | ||
|
||
export interface PureComponent<P = {}, S = {}> extends preact.Component { | ||
export class PureComponent<P = {}, S = {}> extends preact.Component { |
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.
PureComponent
is an actual class that should be extended by other classes, not an interface that any object implement
@@ -67,7 +67,7 @@ declare namespace React { | |||
|
|||
export function unstable_batchedUpdates(callback: (arg?: any) => void, arg?: any): void; | |||
|
|||
export interface Children { | |||
export const Children: { |
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.
Updating the type to be a real object, not an interface for some other object to implement
|
||
export * from 'preact/hooks'; |
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.
Moved all exports to the bottom of the file so they are grouped together
@@ -19,7 +19,7 @@ export interface Component<P = {}, S = {}> extends PreactComponent<P, S> { | |||
|
|||
export interface FunctionalComponent<P = {}> extends PreactFunctionalComponent<P> { | |||
shouldComponentUpdate?(nextProps: Readonly<P>): boolean; | |||
_forwarded?: true; | |||
_forwarded?: boolean; |
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.
Makes VSCode happier when used in compat/index.js
@@ -10,4 +10,4 @@ export interface SuspenseProps { | |||
fallback: preact.ComponentChildren; | |||
} | |||
|
|||
export abstract class Suspense extends Component<SuspenseProps> {} | |||
export class Suspense extends Component<SuspenseProps> {} |
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.
Suspense isn't supposed to be extended so removed the abstract keyword
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.
Awesome🚀😍 Kinda funny that we missed the re-export of both Suspense
and lazy
. Glad to see that cleaned up 👍💯🚀
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.
Great work Andre!
What about the coverage? Also types and tests would maybe be a bit better in separate prs 🤔 |
I think the coverage failing was a fluke. It passed on a second run. And yea, perhaps they are better as separate PRs. Whoops. Next time. |
No description provided.