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

Improve formatting #2066

Merged
merged 8 commits into from
Oct 30, 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
11 changes: 6 additions & 5 deletions compat/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,11 +390,12 @@ function memo(c, comparer) {
if (!updateRef && ref) {
ref.call ? ref(null) : (ref.current = null);
}
return (
(!comparer
? shallowDiffers(this.props, nextProps)
: !comparer(this.props, nextProps)) || !updateRef
);

if (!comparer) {
return shallowDiffers(this.props, nextProps);
}

return !comparer(this.props, nextProps) || !updateRef;
}

function Memoed(props) {
Expand Down
24 changes: 13 additions & 11 deletions compat/test/browser/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -380,17 +380,19 @@ describe('preact-compat', () => {

it('should return null if given null', () => {
expect(findDOMNode(null)).to.be.null;
}),
it('should return a regular DOM Element if given a regular DOM Element', () => {
let scratch = document.createElement('div');
expect(findDOMNode(scratch)).to.equalNode(scratch);
}),
// NOTE: React.render() returning false or null has the component pointing
// to no DOM Node, in contrast, Preact always render an empty Text DOM Node.
it('should return null if render returns false', () => {
const helper = React.render(<Helper something={false} />, scratch);
expect(findDOMNode(helper)).to.be.null;
});
});

it('should return a regular DOM Element if given a regular DOM Element', () => {
let scratch = document.createElement('div');
expect(findDOMNode(scratch)).to.equalNode(scratch);
});

// NOTE: React.render() returning false or null has the component pointing
// to no DOM Node, in contrast, Preact always render an empty Text DOM Node.
it('should return null if render returns false', () => {
const helper = React.render(<Helper something={false} />, scratch);
expect(findDOMNode(helper)).to.be.null;
});

// NOTE: React.render() returning false or null has the component pointing
// to no DOM Node, in contrast, Preact always render an empty Text DOM Node.
Expand Down
29 changes: 15 additions & 14 deletions compat/test/browser/suspense.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,22 @@ import { setupScratch, teardown } from '../../../test/_util/helpers';

function createLazy() {
/** @type {(c: ComponentType) => Promise<void>} */
let resolver, rejecter, promise;
const Lazy = lazy(
() =>
(promise = new Promise((resolve, reject) => {
resolver = c => {
resolve({ default: c });
return promise;
};
let resolver, rejecter;
const Lazy = lazy(() => {
let promise = new Promise((resolve, reject) => {
resolver = c => {
resolve({ default: c });
return promise;
};

rejecter = () => {
reject();
return promise;
};
});

rejecter = () => {
reject();
return promise;
};
}))
);
return promise;
});

return [Lazy, c => resolver(c), e => rejecter(e)];
}
Expand Down
68 changes: 25 additions & 43 deletions debug/src/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export function initDebug() {
if (component && typeof error.then === 'function') {
const promise = error;
error = new Error(
'Missing Suspense. The throwing component was: ' + getDisplayName(vnode)
`Missing Suspense. The throwing component was: ${getDisplayName(vnode)}`
);

let parent = vnode;
Expand All @@ -62,7 +62,8 @@ export function initDebug() {
options._root = (vnode, parentNode) => {
if (!parentNode) {
throw new Error(
'Undefined parent passed to render(), this is the second argument.\nCheck if the element is available in the DOM/has the correct id.'
'Undefined parent passed to render(), this is the second argument.\n' +
'Check if the element is available in the DOM/has the correct id.'
);
}
let isValid;
Expand All @@ -75,13 +76,12 @@ export function initDebug() {
default:
isValid = false;
}
if (!isValid)
throw new Error(`
Expected a valid HTML node as a second argument to render.
Received ${parentNode} instead: render(<${getDisplayName(
vnode
)} />, ${parentNode});
`);
if (!isValid) {
let componentName = getDisplayName(vnode);
throw new Error(
`Expected a valid HTML node as a second argument to render. Received ${parentNode} instead: render(<${componentName} />, ${parentNode});`
);
}

if (oldRoot) oldRoot(vnode, parentNode);
};
Expand All @@ -98,23 +98,12 @@ export function initDebug() {
);
} else if (type != null && typeof type === 'object') {
if (type._lastDomChild !== undefined && type._dom !== undefined) {
let info =
'Did you accidentally pass a JSX literal as JSX twice?\n\n' +
' let My' +
getDisplayName(type) +
' = ' +
serializeVNode(type) +
';\n' +
' let vnode = <My' +
getDisplayName(type) +
' />;\n\n' +
'This usually happens when you export a JSX literal and not the component.';
throw new Error(
'Invalid type passed to createElement(): ' +
type +
'\n\n' +
info +
'\n'
`Invalid type passed to createElement(): ${type}\n\n` +
'Did you accidentally pass a JSX literal as JSX twice?\n\n' +
` let My${getDisplayName(vnode)} = ${serializeVNode(type)};\n` +
` let vnode = <My${getDisplayName(vnode)} />;\n\n` +
'This usually happens when you export a JSX literal and not the component.'
);
}

Expand All @@ -129,8 +118,7 @@ export function initDebug() {
parentVNode.type !== 'table'
) {
console.error(
'Improper nesting of table.' +
'Your <thead/tbody/tfoot> should have a <table> parent.' +
'Improper nesting of table. Your <thead/tbody/tfoot> should have a <table> parent.' +
serializeVNode(vnode)
);
} else if (
Expand All @@ -141,20 +129,17 @@ export function initDebug() {
parentVNode.type !== 'table')
) {
console.error(
'Improper nesting of table.' +
'Your <tr> should have a <thead/tbody/tfoot/table> parent.' +
'Improper nesting of table. Your <tr> should have a <thead/tbody/tfoot/table> parent.' +
serializeVNode(vnode)
);
} else if (type === 'td' && parentVNode.type !== 'tr') {
console.error(
'Improper nesting of table.' +
'Your <td> should have a <tr> parent.' +
'Improper nesting of table. Your <td> should have a <tr> parent.' +
serializeVNode(vnode)
);
} else if (type === 'th' && parentVNode.type !== 'tr') {
console.error(
'Improper nesting of table.' +
'Your <th> should have a <tr>.' +
'Improper nesting of table. Your <th> should have a <tr>.' +
serializeVNode(vnode)
);
}
Expand Down Expand Up @@ -202,7 +187,7 @@ export function initDebug() {
const lazyVNode = vnode.type();
warnedComponents.lazyPropTypes.set(vnode.type, true);
console.warn(
m + 'Component wrapped in lazy() is ' + getDisplayName(lazyVNode)
m + `Component wrapped in lazy() is ${getDisplayName(lazyVNode)}`
);
} catch (promise) {
console.warn(
Expand Down Expand Up @@ -289,10 +274,9 @@ export function initDebug() {
if (Array.isArray(hooks._list)) {
hooks._list.forEach(hook => {
if (hook._callback && (!hook._args || !Array.isArray(hook._args))) {
let componentName = getDisplayName(vnode);
console.warn(
`In ${getDisplayName(
vnode
)} you are calling useMemo/useCallback without passing arguments.\n` +
`In ${componentName} you are calling useMemo/useCallback without passing arguments.\n` +
`This is a noop since it will not be able to memoize, it will execute it every render.`
);
}
Expand All @@ -308,12 +292,11 @@ export function initDebug() {
!warnedComponents.useEffect.has(vnode.type)
) {
warnedComponents.useEffect.set(vnode.type, true);
let componentName = getDisplayName(vnode);
console.warn(
'You should provide an array of arguments as the second argument to the "useEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
'This effect can be found in the render of ' +
getDisplayName(vnode) +
'.'
`This effect can be found in the render of ${componentName}.`
);
}
});
Expand All @@ -328,12 +311,11 @@ export function initDebug() {
!warnedComponents.useLayoutEffect.has(vnode.type)
) {
warnedComponents.useLayoutEffect.set(vnode.type, true);
let componentName = getDisplayName(vnode);
console.warn(
'You should provide an array of arguments as the second argument to the "useLayoutEffect" hook.\n\n' +
'Not doing so will invoke this effect on every render.\n\n' +
'This effect can be found in the render of ' +
getDisplayName(vnode) +
'.'
`This effect can be found in the render of ${componentName}.`
);
}
});
Expand Down
10 changes: 7 additions & 3 deletions debug/src/devtools/custom.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@ export function getNodeType(vnode) {
* @returns {string}
*/
export function getDisplayName(vnode) {
if (vnode.type === Fragment) return 'Fragment';
else if (typeof vnode.type === 'function')
if (vnode.type === Fragment) {
return 'Fragment';
} else if (typeof vnode.type === 'function') {
return vnode.type.displayName || vnode.type.name;
else if (typeof vnode.type === 'string') return vnode.type;
} else if (typeof vnode.type === 'string') {
return vnode.type;
}

return '#text';
}

Expand Down
4 changes: 1 addition & 3 deletions hooks/test/browser/useState.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ describe('useState', () => {
});

it('can initialize the state via a function', () => {
const initState = sinon.spy(() => {
1;
});
const initState = sinon.spy(() => 1);

function Comp() {
useState(initState);
Expand Down
9 changes: 2 additions & 7 deletions karma.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,10 @@ module.exports = function(config) {
captureTimeout: 0,

sauceLabs: {
build:
'CI #' +
process.env.TRAVIS_BUILD_NUMBER +
' (' +
process.env.TRAVIS_BUILD_ID +
')',
build: `CI #${process.env.TRAVIS_BUILD_NUMBER} (${process.env.TRAVIS_BUILD_ID})`,
tunnelIdentifier:
process.env.TRAVIS_JOB_NUMBER ||
'local' + require('./package.json').version,
`local${require('./package.json').version}`,
connectLocationForSERelay: 'localhost',
connectPortForSERelay: 4445,
startConnect: false
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
"license": "MIT",
"types": "src/index.d.ts",
"scripts": {
"build": "npm-run-all --parallel build:*",
"build:core": "microbundle build --raw && cp dist/preact.js dist/preact.min.js",
"build": "npm-run-all --parallel build:* && cp dist/preact.js dist/preact.min.js",
"build:core": "microbundle build --raw",
"build:debug": "microbundle build --raw --cwd debug",
"build:hooks": "microbundle build --raw --cwd hooks",
"build:test-utils": "microbundle build --raw --cwd test-utils",
Expand Down
9 changes: 6 additions & 3 deletions src/diff/children.js
Original file line number Diff line number Diff line change
Expand Up @@ -195,13 +195,16 @@ export function diffChildren(
newParentVNode._dom = firstChildDom;

// Remove children that are not part of any vnode.
if (excessDomChildren != null && typeof newParentVNode.type !== 'function')
for (i = excessDomChildren.length; i--; )
if (excessDomChildren != null && typeof newParentVNode.type !== 'function') {
for (i = excessDomChildren.length; i--; ) {
if (excessDomChildren[i] != null) removeNode(excessDomChildren[i]);
}
}

// Remove remaining oldChildren if there are any.
for (i = oldChildrenLength; i--; )
for (i = oldChildrenLength; i--; ) {
if (oldChildren[i] != null) unmount(oldChildren[i], oldChildren[i]);
}

// Set refs only after unmount
if (refs) {
Expand Down
10 changes: 7 additions & 3 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,10 @@ function diffElementNodes(
}

if (newVNode.type === null) {
if (excessDomChildren != null)
if (excessDomChildren != null) {
excessDomChildren[excessDomChildren.indexOf(dom)] = null;
}

if (oldProps !== newProps) {
dom.data = newProps;
}
Expand Down Expand Up @@ -355,14 +357,16 @@ function diffElementNodes(
'value' in newProps &&
newProps.value !== undefined &&
newProps.value !== dom.value
)
) {
dom.value = newProps.value == null ? '' : newProps.value;
}
if (
'checked' in newProps &&
newProps.checked !== undefined &&
newProps.checked !== dom.checked
)
) {
dom.checked = newProps.checked;
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/diff/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,21 @@ function setProperty(dom, name, value, oldValue, isSvg) {
oldValue = null;
}

if (oldValue)
if (oldValue) {
for (let i in oldValue) {
if (!(value && i in value)) {
setStyle(s, i, '');
}
}
}

if (value)
if (value) {
for (let i in value) {
if (!oldValue || value[i] !== oldValue[i]) {
setStyle(s, i, value[i]);
}
}
}
}
}
// Benchmark for comparison: https://esbench.com/bench/574c954bdb965b9a00965ac6
Expand Down
3 changes: 2 additions & 1 deletion test/browser/performance.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ let { createElement: h, Component, render, hydrate } = require(NODE_ENV ===
const MULTIPLIER = ENABLE_PERFORMANCE ? (coverage ? 5 : 1) : 999999;

// let now = typeof performance!=='undefined' && performance.now ? () => performance.now() : () => +new Date();
if (typeof performance === 'undefined')
if (typeof performance === 'undefined') {
window.performance = { now: () => +new Date() };
}

function loop(iter, time) {
let start = performance.now(),
Expand Down