Skip to content

Commit

Permalink
Merge branch 'master' into feat/warnDeprecatedProps
Browse files Browse the repository at this point in the history
  • Loading branch information
developit committed Apr 5, 2019
2 parents c724fe8 + 98f0d7e commit efc011a
Show file tree
Hide file tree
Showing 9 changed files with 156 additions and 14 deletions.
20 changes: 13 additions & 7 deletions hooks/src/index.js
Expand Up @@ -17,8 +17,7 @@ options.render = vnode => {
currentIndex = 0;

if (!currentComponent.__hooks) return;
currentComponent.__hooks._pendingEffects.forEach(invokeEffect);
currentComponent.__hooks._pendingEffects = [];
currentComponent.__hooks._pendingEffects = handleEffects(currentComponent.__hooks._pendingEffects);
};


Expand All @@ -34,8 +33,7 @@ options.diffed = vnode => {

// TODO: Consider moving to a global queue. May need to move
// this to the `commit` option
hooks._pendingLayoutEffects.forEach(invokeEffect);
hooks._pendingLayoutEffects = [];
hooks._pendingLayoutEffects = handleEffects(hooks._pendingLayoutEffects);
};


Expand Down Expand Up @@ -196,8 +194,7 @@ function flushAfterPaintEffects() {
afterPaintEffects.forEach(component => {
component._afterPaintQueued = false;
if (!component._parentDom) return;
component.__hooks._pendingEffects.forEach(invokeEffect);
component.__hooks._pendingEffects = [];
component.__hooks._pendingEffects = handleEffects(component.__hooks._pendingEffects);
});
afterPaintEffects = [];
}
Expand All @@ -220,12 +217,21 @@ if (typeof window !== 'undefined') {
};
}

function handleEffects(effects) {
effects.forEach(invokeCleanup);
effects.forEach(invokeEffect);
return [];
}

function invokeCleanup(hook) {
if (hook._cleanup) hook._cleanup();
}

/**
* Invoke a Hook's effect
* @param {import('./internal').EffectHookState} hook
*/
function invokeEffect(hook) {
if (hook._cleanup) hook._cleanup();
const result = hook._value();
if (typeof result === 'function') hook._cleanup = result;
}
Expand Down
21 changes: 20 additions & 1 deletion hooks/test/browser/useEffect.test.js
@@ -1,4 +1,4 @@
import { setupRerender } from 'preact/test-utils';
import { act } from 'preact/test-utils';
import { createElement as h, render } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useEffect } from '../../src';
Expand Down Expand Up @@ -62,4 +62,23 @@ describe('useEffect', () => {
expect(callback).to.not.be.called;
});
});

it('Should execute effects in the right order', () => {
let executionOrder = [];
const App = ({ i }) => {
executionOrder = [];
useEffect(() => {
executionOrder.push('action1');
return () => executionOrder.push('cleanup1');
}, [i]);
useEffect(() => {
executionOrder.push('action2');
return () => executionOrder.push('cleanup2');
}, [i]);
return <p>Test</p>;
};
act(() => render(<App i={0} />, scratch));
act(() => render(<App i={2} />, scratch));
expect(executionOrder).to.deep.equal(['cleanup1', 'cleanup2', 'action1', 'action2']);
});
});
21 changes: 20 additions & 1 deletion hooks/test/browser/useLayoutEffect.test.js
@@ -1,4 +1,4 @@
import { setupRerender } from 'preact/test-utils';
import { act } from 'preact/test-utils';
import { createElement as h, render } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useEffectAssertions } from './useEffectAssertions.test';
Expand Down Expand Up @@ -69,4 +69,23 @@ describe('useLayoutEffect', () => {

expect(callback).to.be.calledOnce;
});

it('Should execute layout effects in the right order', () => {
let executionOrder = [];
const App = ({ i }) => {
executionOrder = [];
useLayoutEffect(() => {
executionOrder.push('action1');
return () => executionOrder.push('cleanup1');
}, [i]);
useLayoutEffect(() => {
executionOrder.push('action2');
return () => executionOrder.push('cleanup2');
}, [i]);
return <p>Test</p>;
};
act(() => render(<App i={0} />, scratch));
act(() => render(<App i={2} />, scratch));
expect(executionOrder).to.deep.equal(['cleanup1', 'cleanup2', 'action1', 'action2']);
});
});
4 changes: 3 additions & 1 deletion src/create-element.js
Expand Up @@ -93,7 +93,9 @@ export function coerceToVNode(possibleVNode) {

// Clone vnode if it has already been used. ceviche/#57
if (possibleVNode._dom!=null) {
return createVNode(possibleVNode.type, possibleVNode.props, possibleVNode.text, possibleVNode.key, null);
let vnode = createVNode(possibleVNode.type, possibleVNode.props, possibleVNode.text, possibleVNode.key, null);
vnode._dom = possibleVNode._dom;
return vnode;
}

return possibleVNode;
Expand Down
2 changes: 2 additions & 0 deletions src/diff/index.js
Expand Up @@ -64,6 +64,7 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
if (oldVNode._component) {
c = newVNode._component = oldVNode._component;
clearProcessingException = c._processingException;
newVNode._dom = oldVNode._dom;
}
else {
// Instantiate the new component
Expand Down Expand Up @@ -108,6 +109,7 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
}

if (!force && c.shouldComponentUpdate!=null && c.shouldComponentUpdate(newVNode.props, s, cctx)===false) {
dom = newVNode._dom;
c.props = newVNode.props;
c.state = s;
c._dirty = false;
Expand Down
14 changes: 11 additions & 3 deletions src/index.d.ts
@@ -1,9 +1,11 @@
export = preact;
export as namespace preact;

import "./jsx";
import { JSXInternal } from "./jsx";

declare namespace preact {
export import JSX = JSXInternal;

//
// Preact Virtual DOM
// -----------------------------------
Expand Down Expand Up @@ -134,16 +136,22 @@ declare namespace preact {

function createElement(
type: string,
props: JSX.HTMLAttributes & JSX.SVGAttributes & Record<string, any> | null,
props: JSXInternal.HTMLAttributes & JSXInternal.SVGAttributes & Record<string, any> | null,
...children: ComponentChildren[]
): VNode<any>;
function createElement<P>(
type: ComponentFactory<P>,
props: Attributes & P | null,
...children: ComponentChildren[]
): VNode<any>;
namespace createElement {
export import JSX = JSXInternal;
}

const h: typeof createElement;
type h = typeof createElement;
namespace h {
export import JSX = JSXInternal;
}

//
// Preact render
Expand Down
2 changes: 1 addition & 1 deletion src/jsx.d.ts
Expand Up @@ -7,7 +7,7 @@ type Defaultize<Props, Defaults> =
& Pick<Props, Exclude<keyof Props, keyof Defaults>>
: never;

declare namespace JSX {
export namespace JSXInternal {

type LibraryManagedAttributes<Component, Props> =
Component extends { defaultProps: infer Defaults }
Expand Down
70 changes: 70 additions & 0 deletions test/browser/components.test.js
Expand Up @@ -1252,4 +1252,74 @@ describe('Components', () => {
expect(C3.prototype.componentWillMount, 'inject between, C3 w/ intermediary div').to.have.been.calledOnce;
});
});

it('should set component._vnode._dom when sCU returns false', () => {
let parent;
class Parent extends Component {
render() {
parent = this;
return <Child />;
}
}

let condition = false;

let child;
class Child extends Component {
shouldComponentUpdate() {
return false;
}
render() {
child = this;
if (!condition) return null;
return <div class="child" />;
}
}

let app;
class App extends Component {
render() {
app = this;
return <Parent />;
}
}

render(<App />, scratch);
expect(child._vnode._dom).to.equal(child.base);

app.forceUpdate();
expect(child._vnode._dom).to.equal(child.base);

parent.setState({});
condition = true;
child.forceUpdate();
expect(child._vnode._dom).to.equal(child.base);
rerender();

expect(child._vnode._dom).to.equal(child.base);

condition = false;
app.setState({});
child.forceUpdate();
rerender();
expect(child._vnode._dom).to.equal(child.base);
});

it('should update old dom on forceUpdate in a lifecycle', () => {
let i = 0;
class App extends Component {
componentWillReceiveProps() {
this.forceUpdate();
}
render() {
if (i++==0) return <div>foo</div>;
return <div>bar</div>;
}
}

render(<App />, scratch);
render(<App />, scratch);

expect(scratch.innerHTML).to.equal('<div>bar</div>');
});
});
16 changes: 16 additions & 0 deletions test/ts/jsx-namespacce-test.tsx
@@ -0,0 +1,16 @@
import { createElement, Component } from "../../src";

// declare global JSX types that should not be mixed with preact's internal types
declare global {
namespace JSX {
interface Element {
unknownProperty: string
}
}
}

class SimpleComponent extends Component {
render() {
return <div>It works</div>;
}
}

0 comments on commit efc011a

Please sign in to comment.