Skip to content

Commit

Permalink
Merge branch 'master' into feat/warnUsingHooksOutsideRender
Browse files Browse the repository at this point in the history
  • Loading branch information
marvinhagemeister committed Apr 20, 2019
2 parents 1d0027a + 9e07832 commit 4f6db67
Show file tree
Hide file tree
Showing 21 changed files with 351 additions and 56 deletions.
2 changes: 1 addition & 1 deletion debug/src/devtools/custom.js
Expand Up @@ -145,7 +145,7 @@ export function getInstance(vnode) {
if (isRoot(vnode)) {
// Edge case: When the tree only consists of components that have not rendered
// anything into the DOM we revert to using the vnode as instance.
return vnode._children.length > 0 && vnode._children[0]._dom!=null
return vnode._children.length > 0 && vnode._children[0]!=null && vnode._children[0]._dom!=null
? /** @type {import('../internal').PreactElement | null} */
(vnode._children[0]._dom.parentNode)
: vnode;
Expand Down
6 changes: 3 additions & 3 deletions debug/test/browser/devtools.test.js
Expand Up @@ -681,14 +681,14 @@ describe('devtools', () => {
rerender();
checkEventReferences(prev.concat(hook.log));

// We swap unkeyed children if the match by type. In this case we'll
// We swap unkeyed children if they match by type. In this case we'll
// use `<Foo>bar</Foo>` as the old child to diff against for
// `<Foo>foo</Foo>`. That's why `<Foo>bar</Foo>` needs to be remounted.
expect(serialize(hook.log)).to.deep.equal([
{ type: 'update', component: 'Foo' },
{ type: 'mount', component: '#text: bar' },
{ type: 'mount', component: '#text: foo' },
{ type: 'mount', component: 'div' },
{ type: 'mount', component: 'Foo' },
{ type: 'update', component: 'Foo' },
{ type: 'update', component: 'App' },
{ type: 'rootCommitted', component: 'Fragment' }
]);
Expand Down
1 change: 0 additions & 1 deletion demo/index.js
Expand Up @@ -29,7 +29,6 @@ if (!isBenchmark) {
window.setImmediate = setTimeout;

class Home extends Component {
a = 1;
render() {
return (
<div>
Expand Down
8 changes: 8 additions & 0 deletions hooks/src/index.d.ts
Expand Up @@ -98,3 +98,11 @@ export function useMemo<T>(factory: () => T, inputs?: Inputs): T;
* @param context The context you want to use
*/
export function useContext<T>(context: PreactContext<T>): T;

/**
* Customize the displayed value in the devtools panel.
*
* @param value Custom hook name or object that is passed to formatter
* @param formatter Formatter to modify value before sending it to the devtools
*/
export function useDebugValue<T>(value: T, formatter?: (value: T) => string | number): void;
15 changes: 10 additions & 5 deletions hooks/src/index.js
Expand Up @@ -109,7 +109,6 @@ export function useEffect(callback, args) {
state._args = args;

currentComponent.__hooks._pendingEffects.push(state);
if (Array.isArray(options.effects)) options.effects.push(state);
afterPaint(currentComponent);
}
}
Expand All @@ -125,7 +124,6 @@ export function useLayoutEffect(callback, args) {
if (argsChanged(state._args, args)) {
state._value = callback;
state._args = args;
if (Array.isArray(options.effects)) options.effects.push(state);
currentComponent.__hooks._pendingLayoutEffects.push(state);
}
}
Expand Down Expand Up @@ -178,6 +176,16 @@ export function useContext(context) {
return provider.props.value;
}

/**
* Display a custom label for a custom hook for the devtools panel
* @type {<T>(value: T, cb?: (value: T) => string | number) => void}
*/
export function useDebugValue(value, formatter) {
if (options.useDebugValue) {
options.useDebugValue(formatter ? formatter(value) : value);
}
}

// Note: if someone used Component.debounce = requestAnimationFrame,
// then effects will ALWAYS run on the NEXT frame instead of the current one, incurring a ~16ms delay.
// Perhaps this is not such a big deal.
Expand Down Expand Up @@ -220,9 +228,6 @@ if (typeof window !== 'undefined') {
function handleEffects(effects) {
effects.forEach(invokeCleanup);
effects.forEach(invokeEffect);
if (options.effects) {
effects.forEach(hook => options.effects = options.effects.filter(h => h!==hook));
}
return [];
}

Expand Down
44 changes: 42 additions & 2 deletions hooks/test/browser/combinations.test.js
@@ -1,5 +1,5 @@
import { setupRerender } from 'preact/test-utils';
import { createElement as h, render } from 'preact';
import { setupRerender, act } from 'preact/test-utils';
import { createElement as h, render, Component } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useState, useReducer, useEffect, useLayoutEffect, useRef } from '../../src';
import { scheduleEffectAssert } from '../_util/useEffectUtil';
Expand Down Expand Up @@ -174,4 +174,44 @@ describe('combinations', () => {
expect(effectCount).to.equal(1);
});
});

it('should not reuse functional components with hooks', () => {
let updater = { first: undefined, second: undefined };
function Foo(props) {
let [v, setter] = useState(0);
updater[props.id] = () => setter(++v);
return <div>{v}</div>;
}

let updateParent;
class App extends Component {
constructor(props) {
super(props);
this.state = { active: true };
updateParent = () => this.setState(p => ({ active: !p.active }));
}

render() {
return (
<div>
{this.state.active && <Foo id="first" />}
<Foo id="second" />
</div>
);
}
}

render(<App />, scratch);
act(() => updater.second());
expect(scratch.textContent).to.equal('01');

updateParent();
rerender();
expect(scratch.textContent).to.equal('1');

updateParent();
rerender();

expect(scratch.textContent).to.equal('01');
});
});
72 changes: 72 additions & 0 deletions hooks/test/browser/useDebugValue.test.js
@@ -0,0 +1,72 @@
import { h, render, options } from 'preact';
import { setupScratch, teardown } from '../../../test/_util/helpers';
import { useDebugValue, useState } from '../../src';

/** @jsx h */

describe('useDebugValue', () => {

/** @type {HTMLDivElement} */
let scratch;

beforeEach(() => {
scratch = setupScratch();
});

afterEach(() => {
teardown(scratch);
delete options.useDebugValue;
});

it('should do nothing when no options hook is present', () => {
function useFoo() {
useDebugValue('foo');
return useState(0);
}

function App() {
let [v] = useFoo();
return <div>{v}</div>;
}

expect(() => render(<App />, scratch)).to.not.throw();
});

it('should call options hook with value', () => {
let spy = options.useDebugValue = sinon.spy();

function useFoo() {
useDebugValue('foo');
return useState(0);
}

function App() {
let [v] = useFoo();
return <div>{v}</div>;
}

render(<App />, scratch);

expect(spy).to.be.calledOnce;
expect(spy).to.be.calledWith('foo');
});

it('should apply optional formatter', () => {
let spy = options.useDebugValue = sinon.spy();

function useFoo() {
useDebugValue('foo', x => x + 'bar');
return useState(0);
}

function App() {
let [v] = useFoo();
return <div>{v}</div>;
}

render(<App />, scratch);

expect(spy).to.be.calledOnce;
expect(spy).to.be.calledWith('foobar');
});
});
4 changes: 3 additions & 1 deletion mangle.json
Expand Up @@ -17,6 +17,7 @@
"props": {
"cname": 6,
"props": {
"$_depth": "__b",
"$_dirty": "__d",
"$_nextState": "__s",
"$_renderCallbacks": "__h",
Expand All @@ -31,7 +32,8 @@
"$_context": "__n",
"$_defaultValue": "__p",
"$_id": "__c",
"$_parentDom": "__P"
"$_parentDom": "__P",
"$_self": "_"
}
}
}
2 changes: 1 addition & 1 deletion package.json
@@ -1,7 +1,7 @@
{
"name": "preact",
"amdName": "preact",
"version": "10.0.0-alpha.4",
"version": "10.0.0-beta.0",
"private": false,
"description": "Fast 3kb React-compatible Virtual DOM library.",
"main": "dist/preact.js",
Expand Down
1 change: 1 addition & 0 deletions src/create-element.js
Expand Up @@ -62,6 +62,7 @@ export function createVNode(type, props, text, key, ref) {
_lastDomChild: null,
_component: null
};
vnode._self = vnode;

if (options.vnode) options.vnode(vnode);

Expand Down
41 changes: 26 additions & 15 deletions src/diff/children.js
Expand Up @@ -27,7 +27,7 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
let childVNode, i, j, p, index, oldVNode, newDom,
nextDom, sibDom, focus;

let newChildren = newParentVNode._children || toChildArray(newParentVNode.props.children, newParentVNode._children=[], coerceToVNode);
let newChildren = newParentVNode._children || toChildArray(newParentVNode.props.children, newParentVNode._children=[], coerceToVNode, true);
let oldChildren = oldParentVNode!=null && oldParentVNode!=EMPTY_OBJ && oldParentVNode._children || EMPTY_ARR;

let oldChildrenLength = oldChildren.length;
Expand Down Expand Up @@ -63,16 +63,19 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
// Check if we find a corresponding element in oldChildren and store the
// index where the element was found.
p = oldChildren[i];
if (p != null && (childVNode.key==null && p.key==null ? (childVNode.type === p.type) : (childVNode.key === p.key))) {
index = i;
}
else {
for (j=0; j<oldChildrenLength; j++) {
p = oldChildren[j];
if (p!=null) {
if (childVNode.key==null && p.key==null ? (childVNode.type === p.type) : (childVNode.key === p.key)) {
index = j;
break;

if (childVNode!=null) {
if (p===null || (p != null && (childVNode.key==null && p.key==null ? (childVNode.type === p.type) : (childVNode.key === p.key)))) {
index = i;
}
else {
for (j=0; j<oldChildrenLength; j++) {
p = oldChildren[j];
if (p!=null) {
if (childVNode.key==null && p.key==null ? (childVNode.type === p.type) : (childVNode.key === p.key)) {
index = j;
break;
}
}
}
}
Expand All @@ -83,7 +86,9 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
// element.
if (index!=null) {
oldVNode = oldChildren[index];
oldChildren[index] = null;
// We can't use `null` here because that is reserved for empty
// placeholders (holes)
oldChildren[index] = undefined;
}

nextDom = oldDom!=null && oldDom.nextSibling;
Expand Down Expand Up @@ -146,13 +151,19 @@ export function diffChildren(parentDom, newParentVNode, oldParentVNode, context,
* @param {import('../index').ComponentChildren} children The unflattened
* children of a virtual node
* @param {Array<import('../internal').VNode | null>} [flattened] An flat array of children to modify
* @param {typeof import('../create-element').coerceToVNode} [map] Function that
* will be applied on each child if the `vnode` is not `null`
* @param {boolean} [keepHoles] wether to coerce `undefined` to `null` or not.
* This is needed for Components without children like `<Foo />`.
*/
export function toChildArray(children, flattened, map) {
export function toChildArray(children, flattened, map, keepHoles) {
if (flattened == null) flattened = [];
if (children==null || typeof children === 'boolean') {}
if (children==null || typeof children === 'boolean') {
if (keepHoles) flattened.push(null);
}
else if (Array.isArray(children)) {
for (let i=0; i < children.length; i++) {
toChildArray(children[i], flattened);
toChildArray(children[i], flattened, map, keepHoles);
}
}
else {
Expand Down
13 changes: 9 additions & 4 deletions src/diff/index.js
Expand Up @@ -34,6 +34,11 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
oldVNode = EMPTY_OBJ;
}

// When passing through createElement it assigns the object
// ref on _self, to prevent JSON Injection we check if this attribute
// is equal.
if (newVNode._self!==newVNode) return null;

if (options.diff) options.diff(newVNode);

let c, p, isNew = false, oldProps, oldState, snapshot,
Expand All @@ -50,7 +55,7 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD
// we'll set `dom` to the correct value just a few lines later.
dom = null;

if (newVNode._children.length) {
if (newVNode._children.length && newVNode._children[0]!=null) {
dom = newVNode._children[0]._dom;

// If the last child is a Fragment, use _lastDomChild, else use _dom
Expand All @@ -70,7 +75,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;
dom = newVNode._dom = oldVNode._dom;
}
else {
// Instantiate the new component
Expand Down Expand Up @@ -137,7 +142,7 @@ export function diff(dom, parentDom, newVNode, oldVNode, context, isSvg, excessD

if (options.render) options.render(newVNode);

let prev = c._prevVNode;
let prev = c._prevVNode || null;
let vnode = c._prevVNode = coerceToVNode(c.render(c.props, c.state, c.context));
c._dirty = false;

Expand Down Expand Up @@ -347,7 +352,7 @@ export function unmount(vnode, ancestorComponent, skipRemove) {
}
else if (r = vnode._children) {
for (let i = 0; i < r.length; i++) {
unmount(r[i], ancestorComponent, skipRemove);
if (r[i]) unmount(r[i], ancestorComponent, skipRemove);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/index.d.ts
Expand Up @@ -196,6 +196,7 @@ declare namespace preact {
/** Attach a hook that is invoked after a vnode has rendered. */
diffed?(vnode: VNode): void;
event?(e: Event): void;
useDebugValue?(value: string | number): void;
}

const options: Options;
Expand Down

0 comments on commit 4f6db67

Please sign in to comment.