Skip to content

Optimizations 1 #3377

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

Draft
wants to merge 17 commits into
base: v11
Choose a base branch
from
Draft
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
4 changes: 2 additions & 2 deletions compat/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ function findDOMNode(component) {
return component;
}

return getChildDom(component._internal);
return getChildDom(component._internal, 0);
}

/**
Expand Down Expand Up @@ -120,7 +120,7 @@ const StrictMode = Fragment;
* @param {Arg} [arg] Optional arugment that can be passed to the callback
* @returns
*/
const flushSync = (callback, arg) => callback(arg)
const flushSync = (callback, arg) => callback(arg);

export * from 'preact/hooks';
export {
Expand Down
2 changes: 1 addition & 1 deletion hooks/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ export function useReducer(reducer, initialState, init) {
const nextValue = hookState._reducer(hookState._value[0], action);
if (hookState._value[0] !== nextValue) {
hookState._value = [nextValue, hookState._value[1]];
hookState._internal.rerender(hookState._internal);
hookState._internal.render();
}
}
];
Expand Down
18 changes: 6 additions & 12 deletions jsx-runtime/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ function createVNode(type, props, key, __source, __self) {
// forwardRef components in the future, but that should happen via
// a separate PR.
let normalizedProps = {};
let ref;
for (let i in props) {
if (i != 'ref') {
if (i === 'ref') {
ref = props[i];
} else {
normalizedProps[i] = props[i];
}
}
Expand All @@ -38,24 +41,15 @@ function createVNode(type, props, key, __source, __self) {
type,
props: normalizedProps,
key,
ref: props && props.ref,
ref,
constructor: undefined,
_vnodeId: --vnodeId,
__source,
__self
};

// If a Component VNode, check for and apply defaultProps.
// Note: `type` is often a String, and can be `undefined` in development.
let defaults, i;
if (typeof type === 'function' && (defaults = type.defaultProps)) {
for (i in defaults)
if (normalizedProps[i] === undefined) {
normalizedProps[i] = defaults[i];
}
}

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

return vnode;
}

Expand Down
6 changes: 4 additions & 2 deletions jsx-runtime/test/browser/jsx-runtime.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ describe('Babel jsx/jsxDEV', () => {
expect(vnode.key).to.equal('foo');
});

it('should apply defaultProps', () => {
// We no longer support defaultProps.
it.skip('should apply defaultProps', () => {
class Foo extends Component {
render() {
return <div />;
Expand All @@ -48,7 +49,7 @@ describe('Babel jsx/jsxDEV', () => {
});
});

it('should keep props over defaultProps', () => {
it.skip('should keep props over defaultProps', () => {
class Foo extends Component {
render() {
return <div />;
Expand Down Expand Up @@ -81,6 +82,7 @@ describe('Babel jsx/jsxDEV', () => {
delete jsxVNode.__source;
delete jsxVNode._vnodeId;
delete elementVNode._vnodeId;
// expect(jsxVNode.constructor).to.equal(elementVNode.constructor);
expect(jsxVNode).to.deep.equal(elementVNode);
});

Expand Down
1 change: 1 addition & 0 deletions mangle.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"props": {
"cname": 6,
"props": {
"$flags": "f",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would opt to keep this to flags to encourage use of library authors for additional extensions that we don't want to support first-party

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been going back and forth on it. It pains me that this is 23b just for lags 👁

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, the community being able to use this without looking at this file kind off boats well for me but yeah, one opinion 😅 also the types won't ever reflect .f unless we start doing two different type files

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true about types too (though that makes me think we should probably already be doing internal VS external files given we have other minified properties)

"$_vnodeId": "__v",
"$_cleanup": "__c",
"$_afterPaintQueued": "__a",
Expand Down
31 changes: 6 additions & 25 deletions src/clone-element.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createVNode } from './create-element';
import { EMPTY_ARR } from './constants';
import { createElement } from './create-element';

/**
* Clones the given VNode, optionally adding attributes/props and replacing its children.
Expand All @@ -8,33 +9,13 @@ import { createVNode } from './create-element';
* @returns {import('./internal').VNode}
*/
export function cloneElement(vnode, props, children) {
let normalizedProps = Object.assign({}, vnode.props),
key,
ref,
i;

for (i in props) {
if (i == 'key') key = props[i];
else if (i == 'ref') ref = props[i];
else normalizedProps[i] = props[i];
}

if (arguments.length > 3) {
children = [children];
for (i = 3; i < arguments.length; i++) {
children.push(arguments[i]);
}
}

if (arguments.length > 2) {
normalizedProps.children = children;
children = EMPTY_ARR.slice.call(arguments, 2);
}

return createVNode(
return createElement(
vnode.type,
normalizedProps,
key || vnode.key,
ref || vnode.ref,
0
Object.assign({ key: vnode.key, ref: vnode.ref }, vnode.props, props),
children
);
}
117 changes: 64 additions & 53 deletions src/component.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { addCommitCallback, commitRoot } from './diff/commit';
import { commitRoot } from './diff/commit';
import options from './options';
import { createVNode, Fragment } from './create-element';
import { patch } from './diff/patch';
Expand Down Expand Up @@ -47,35 +47,39 @@ Component.prototype.setState = function(update, callback) {
update = update(Object.assign({}, s), this.props);
}

if (update) {
Object.assign(s, update);
}
Object.assign(s, update);

// Skip update if updater function returned null
if (update == null) return;

if (this._internal) {
if (callback) addCommitCallback(this._internal, callback.bind(this));
this._internal.rerender(this._internal);
}
// The 0 flag value here prevents FORCE_UPDATE from being set
renderComponentInstance.call(this, callback, 0);
};

/**
* Immediately perform a synchronous re-render of the component
* @param {() => void} [callback] A function to call after re-rendering completes
* @this {import('./internal').Component}
*/
Component.prototype.forceUpdate = renderComponentInstance;

/**
* Immediately perform a synchronous re-render of the component.
* This method is the implementation of forceUpdate() for class components.
* @param {() => void} [callback] A function to call after rendering completes
* @param {number} [flags = FORCE_UPDATE] Flags to set. Defaults to FORCE_UPDATE.
* @this {import('./internal').Component}
* @param {() => void} [callback] A function to be called after component is
* re-rendered
*/
Component.prototype.forceUpdate = function(callback) {
export function renderComponentInstance(callback, flags) {
if (this._internal) {
// Set render mode so that we can differentiate where the render request
// is coming from. We need this because forceUpdate should never call
// shouldComponentUpdate
this._internal.flags |= FORCE_UPDATE;
if (callback) addCommitCallback(this._internal, callback.bind(this));
this._internal.rerender(this._internal);
// is coming from (eg: forceUpdate should never call shouldComponentUpdate).
this._internal.flags |= flags == null ? FORCE_UPDATE : flags;
this._internal.render(callback);
// Note: the above is equivalent to invoking enqueueRender:
// enqueueRender.call(this._internal, callback);
}
};
}

/**
* Accepts `props` and `state`, and returns a new Virtual DOM tree to build.
Expand All @@ -90,36 +94,36 @@ Component.prototype.forceUpdate = function(callback) {
Component.prototype.render = Fragment;

/**
* @param {import('./internal').Component} internal The internal to rerender
* Render an Internal that has been marked
* @param {import('./internal').Internal} internal The internal to rerender
*/
function rerender(internal) {
if (~internal.flags & MODE_UNMOUNTING && internal.flags & DIRTY_BIT) {
let parentDom = getParentDom(internal);
let startDom =
(internal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) ===
(MODE_HYDRATE | MODE_SUSPENDED)
? internal._dom
: getDomSibling(internal, 0);

const vnode = createVNode(
internal.type,
internal.props,
internal.key, // @TODO we shouldn't need to actually pass these
internal.ref, // since the mode flag should bypass key/ref handling
0
);

const commitQueue = [];
patch(parentDom, vnode, internal, commitQueue, startDom);
commitRoot(commitQueue, internal);
}
}
const renderQueuedInternal = internal => {
const commitQueue = [];

const vnode = createVNode(internal.type, internal.props);

// Don't render unmounting/unmounted trees:
if (internal.flags & MODE_UNMOUNTING) return;

// Don't render trees already rendered in this pass:
if (!(internal.flags & DIRTY_BIT)) return;

let parentDom = getParentDom(internal);
let startDom =
(internal.flags & (MODE_HYDRATE | MODE_SUSPENDED)) ===
(MODE_HYDRATE | MODE_SUSPENDED)
? internal._dom
: getDomSibling(internal, 0);

patch(parentDom, vnode, internal, commitQueue, startDom);
commitRoot(commitQueue, internal);
};

/**
* The render queue
* @type {Array<import('./internal').Component>}
* A queue of Internals to be rendered in the next batch.
* @type {Array<import('./internal').Internal>}
*/
let rerenderQueue = [];
let renderQueue = [];

/*
* The value of `Component.debounce` must asynchronously invoke the passed in callback. It is
Expand All @@ -136,28 +140,35 @@ const defer = Promise.prototype.then.bind(Promise.resolve());

/**
* Enqueue a rerender of a component
* @param {import('./internal').Component} internal The internal to rerender
* @this {import('./internal').Internal} internal The internal to rerender
*/
export function enqueueRender(internal) {
export function enqueueRender(callback) {
let internal = this;
if (callback) {
if (internal._commitCallbacks == null) {
internal._commitCallbacks = [];
}
internal._commitCallbacks.push(callback);
}
if (
(!(internal.flags & DIRTY_BIT) &&
(internal.flags |= DIRTY_BIT) &&
rerenderQueue.push(internal) &&
!process._rerenderCount++) ||
renderQueue.push(internal) &&
!processRenderQueue._rerenderCount++) ||
prevDebounce !== options.debounceRendering
) {
prevDebounce = options.debounceRendering;
(prevDebounce || defer)(process);
(prevDebounce || defer)(processRenderQueue);
}
}

/** Flush the render queue by rerendering all queued components */
function process() {
while ((len = process._rerenderCount = rerenderQueue.length)) {
rerenderQueue.sort((a, b) => a._depth - b._depth);
function processRenderQueue() {
while ((len = processRenderQueue._rerenderCount = renderQueue.length)) {
renderQueue.sort((a, b) => a._depth - b._depth);
while (len--) {
rerender(rerenderQueue.shift());
renderQueuedInternal(renderQueue.shift());
}
}
}
let len = (process._rerenderCount = 0);
let len = (processRenderQueue._rerenderCount = 0);
11 changes: 3 additions & 8 deletions src/create-context.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { enqueueRender } from './component';

let nextContextId = 0;

const providers = new Set();
Expand All @@ -9,7 +7,7 @@ export const unsubscribeFromContext = internal => {
if (providers.delete(internal)) return;
// ... otherwise, unsubscribe from any contexts:
providers.forEach(p => {
p._component._subs.delete(internal);
p._subs.delete(internal);
});
};

Expand All @@ -21,9 +19,6 @@ export const createContext = (defaultValue, contextId) => {
_defaultValue: defaultValue,
/** @type {import('./internal').FunctionComponent} */
Consumer(props, contextValue) {
// return props.children(
// context[contextId] ? context[contextId].props.value : defaultValue
// );
return props.children(contextValue);
},
/** @type {import('./internal').FunctionComponent} */
Expand All @@ -34,11 +29,11 @@ export const createContext = (defaultValue, contextId) => {
ctx = {};
ctx[contextId] = this;
this.getChildContext = () => ctx;
providers.add(this._internal);
providers.add(this);
}
// re-render subscribers in response to value change
else if (props.value !== this._prev) {
this._subs.forEach(enqueueRender);
this._subs.forEach(i => i.render());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
this._subs.forEach(i => i.render());
this._subs.forEach(i => { i.render() });

Saves some bytes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha! can't believe I missed that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoviDeCroock would you believe that actually adds 3b?! gzip

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it comes from the return keyword added at build time 🤷

}
this._prev = props.value;

Expand Down
Loading