Skip to content
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

Merged
merged 9 commits into from
Jun 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions compat/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<any>["type"]): (props?: any, ...children: preact.ComponentChildren[]) => preact.VNode<any>;
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 {
Copy link
Member Author

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

isPureReactComponenet: boolean;
}

Expand All @@ -67,7 +67,7 @@ declare namespace React {

export function unstable_batchedUpdates(callback: (arg?: any) => void, arg?: any): void;

export interface Children {
export const Children: {
Copy link
Member Author

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

map<T extends preact.ComponentChild, R>(children: T | T[], fn: (child: T, i: number, array: T[]) => R): R[];
forEach<T extends preact.ComponentChild>(children: T | T[], fn: (child: T, i: number, array: T[]) => void): void;
count: (children: preact.ComponentChildren) => number;
Expand Down
31 changes: 15 additions & 16 deletions compat/src/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { hydrate, render as preactRender, cloneElement as preactCloneElement, createRef, h, Component, options, toChildArray, createContext, Fragment } from 'preact';
import * as hooks from 'preact/hooks';
export * from 'preact/hooks';
import { Suspense as _Suspense, lazy as _lazy, catchRender } from './suspense';
import { Suspense, lazy, catchRender } from './suspense';
import { assign, removeNode } from '../../src/util';

const version = '16.8.0'; // trick libraries to think we are react
Expand Down Expand Up @@ -174,12 +173,11 @@ function normalizeVNode(vnode) {
* all vnode normalizations.
* @param {import('./internal').VNode} element The vnode to clone
* @param {object} props Props to add when cloning
* @param {Array<import('./internal').ComponentChildren} rest Optional component children
* @param {Array<import('./internal').ComponentChildren>} rest Optional component children
*/
function cloneElement(element) {
if (!isValidElement(element)) return element;
let vnode = normalizeVNode(preactCloneElement.apply(null, arguments));
vnode.$$typeof = REACT_ELEMENT_TYPE;
Copy link
Member Author

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

return vnode;
}

Expand Down Expand Up @@ -222,7 +220,7 @@ function applyEventNormalization({ type, props }) {

/**
* Remove a component tree from the DOM, including state and event handlers.
* @param {Element | Document | ShadowRoot | DocumentFragment} container
* @param {import('./internal').PreactElement} container
* @returns {boolean}
*/
function unmountComponentAtNode(container) {
Expand Down Expand Up @@ -287,7 +285,7 @@ class PureComponent extends Component {
}
}

// Some libraries like `react-virtualized` explicitely check for this.
// Some libraries like `react-virtualized` explicitly check for this.
Component.prototype.isReactComponent = {};

/**
Expand Down Expand Up @@ -366,14 +364,14 @@ options.vnode = vnode => {
/**
* Deprecated way to control batched rendering inside the reconciler, but we
* already schedule in batches inside our rendering code
* @param {(a) => void} callback function that triggers the updatd
* @param {*} [arg] Optional argument that can be passed to the callback
* @template Arg
* @param {(arg: Arg) => void} callback function that triggers the updated
* @param {Arg} [arg] Optional argument that can be passed to the callback
*/
// eslint-disable-next-line camelcase
function unstable_batchedUpdates(callback, arg) {
callback(arg);
}
const unstable_batchedUpdates = (callback, arg) => callback(arg);

export * from 'preact/hooks';
Copy link
Member Author

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

export {
version,
Children,
Expand All @@ -394,12 +392,11 @@ export {
memo,
forwardRef,
// eslint-disable-next-line camelcase
unstable_batchedUpdates
unstable_batchedUpdates,
Suspense,
lazy
};

export const Suspense = _Suspense;
export const lazy = _lazy;

// React copies the named exports to the default one.
export default assign({
version,
Expand All @@ -420,5 +417,7 @@ export default assign({
PureComponent,
memo,
forwardRef,
unstable_batchedUpdates
unstable_batchedUpdates,
Suspense,
lazy
}, hooks);
2 changes: 1 addition & 1 deletion compat/src/internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member Author

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

}

export interface VNode<T = any> extends PreactVNode<T> {
Expand Down
2 changes: 1 addition & 1 deletion compat/src/suspense.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ export interface SuspenseProps {
fallback: preact.ComponentChildren;
}

export abstract class Suspense extends Component<SuspenseProps> {}
export class Suspense extends Component<SuspenseProps> {}
Copy link
Member Author

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

28 changes: 18 additions & 10 deletions compat/test/browser/exports.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ describe('exports', () => {
expect(Compat.useCallback).to.be.a('function');
expect(Compat.useContext).to.be.a('function');

// Suspense
expect(Compat.Suspense).to.be.a('function');
expect(Compat.lazy).to.be.a('function');

// Compat specific
expect(Compat.PureComponent).to.exist.and.be.a('function');
expect(Compat.createPortal).to.exist.and.be.a('function');
Expand Down Expand Up @@ -50,14 +54,18 @@ describe('exports', () => {
expect(Named.createRef).to.be.a('function');

// Hooks
expect(Compat.useState).to.be.a('function');
expect(Compat.useReducer).to.be.a('function');
expect(Compat.useEffect).to.be.a('function');
expect(Compat.useLayoutEffect).to.be.a('function');
expect(Compat.useRef).to.be.a('function');
expect(Compat.useMemo).to.be.a('function');
expect(Compat.useCallback).to.be.a('function');
expect(Compat.useContext).to.be.a('function');
expect(Named.useState).to.be.a('function');
expect(Named.useReducer).to.be.a('function');
expect(Named.useEffect).to.be.a('function');
expect(Named.useLayoutEffect).to.be.a('function');
expect(Named.useRef).to.be.a('function');
expect(Named.useMemo).to.be.a('function');
expect(Named.useCallback).to.be.a('function');
expect(Named.useContext).to.be.a('function');

// Suspense
expect(Named.Suspense).to.be.a('function');
expect(Named.lazy).to.be.a('function');

// Compat specific
expect(Named.PureComponent).to.exist.and.be.a('function');
Expand All @@ -70,8 +78,8 @@ describe('exports', () => {
expect(Named.Children.count).to.exist.and.be.a('function');
expect(Named.Children.toArray).to.exist.and.be.a('function');
expect(Named.Children.only).to.exist.and.be.a('function');
expect(Compat.unmountComponentAtNode).to.exist.and.be.a('function');
expect(Compat.unstable_batchedUpdates).to.exist.and.be.a('function');
expect(Named.unmountComponentAtNode).to.exist.and.be.a('function');
expect(Named.unstable_batchedUpdates).to.exist.and.be.a('function');
expect(Named.version).to.exist.and.be.a('string');
});
});
10 changes: 5 additions & 5 deletions compat/test/browser/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,14 @@ describe('preact-compat', () => {

describe('createFactory', () => {
it('should create a DOM element', () => {
render(createFactory('span', null)(), scratch);
expect(scratch.firstChild.nodeName).to.equal('SPAN');
render(createFactory('span')({ class: 'foo' }, '1'), scratch);
expect(scratch.innerHTML).to.equal('<span class="foo">1</span>');
});

it('should create a component', () => {
const Foo = () => <div>foo</div>;
render(createFactory(Foo, null)(), scratch);
expect(scratch.textContent).to.equal('foo');
const Foo = ({ id, children }) => <div id={id}>foo {children}</div>;
render(createFactory(Foo)({ id: 'value' }, 'bar'), scratch);
expect(scratch.innerHTML).to.equal('<div id="value">foo bar</div>');
});
});

Expand Down
2 changes: 1 addition & 1 deletion src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ declare namespace preact {
replaceNode?: Element | Text
): void;
function hydrate(vnode: ComponentChild, parent: Element | Document | ShadowRoot | DocumentFragment): void;
function cloneElement(vnode: JSX.Element, props: any, ...children: ComponentChildren[]): JSX.Element;
function cloneElement(vnode: JSX.Element, props?: any, ...children: ComponentChildren[]): JSX.Element;

//
// Preact Built-in Components
Expand Down