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 timing marks, update internal variables Close GH-198 #216

Merged
merged 1 commit into from
Mar 25, 2021
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 packages/diffhtml/lib/inner-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@ import { EMPTY, ValidInput, TransactionConfig, Mount } from './util/types';

/**
*
* @param {Mount} domNode
* @param {Mount} mount
* @param {ValidInput} input
* @param {TransactionConfig} config
*
* @return {Promise<Transaction> | unknown}
*/
export default function innerHTML(domNode, input = EMPTY.STR, config = {}) {
export default function innerHTML(mount, input = EMPTY.STR, config = {}) {
config.inner = true;
config.executeScripts = 'executeScripts' in config ? config.executeScripts : true;
config.tasks = config.tasks || defaultTasks;

return Transaction.create(domNode, input, config).start();
return Transaction.create(mount, input, config).start();
}
12 changes: 6 additions & 6 deletions packages/diffhtml/lib/release.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import { StateCache, NodeCache, ReleaseHookCache, Mount } from './util/types';
/**
* Releases state and memory associated to a DOM Node.
*
* @param {Mount} domNode - Valid input node
* @param {Mount} mount - Valid input node
*/
export default function release(domNode) {
export default function release(mount) {
// Try and find a state object for this DOM Node.
const state = StateCache.get(domNode);
const state = StateCache.get(mount);

// If this was a top-level rendered element, deallocate the VTree
// and remove the StateCache reference.
if (state) {
StateCache.delete(domNode);
StateCache.delete(mount);

// If there is a known root association that is not in the NodeCache,
// remove this VTree.
Expand All @@ -25,11 +25,11 @@ export default function release(domNode) {

// The rest of this function only pertains to real HTML element nodes. If
// this is undefined, then it isn't one.
if (!domNode) {
if (!mount) {
return;
}

const asHTMLElement = /** @type {HTMLElement} */(domNode);
const asHTMLElement = /** @type {HTMLElement} */(mount);

// Crawl the childNodes if this is an HTMLElement for state trees.
if (asHTMLElement.childNodes && asHTMLElement.childNodes.length) {
Expand Down
2 changes: 1 addition & 1 deletion packages/diffhtml/lib/tasks/parse-new-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import Transaction from '../transaction';
* @param {Transaction} transaction
*/
export default function parseNewTree(transaction) {
const { state, input, options } = transaction;
const { state, input, config: options } = transaction;
const { measure } = state;
const { inner } = options;

Expand Down
4 changes: 2 additions & 2 deletions packages/diffhtml/lib/tasks/patch-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ import globalThis from '../util/global';
* @return {void}
*/
export default function patch(transaction) {
const { domNode, state, state: { measure, scriptsToExecute }, patches } = transaction;
const { mount, state, state: { measure, scriptsToExecute }, patches } = transaction;

measure('patch node');

const { ownerDocument } = /** @type {HTMLElement} */ (domNode);
const { ownerDocument } = /** @type {HTMLElement} */ (mount);
const promises = transaction.promises || [];

state.ownerDocument = ownerDocument || globalThis.document;
Expand Down
12 changes: 6 additions & 6 deletions packages/diffhtml/lib/tasks/reconcile-trees.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ import release from '../release';
* @param {Transaction} transaction
*/
export default function reconcileTrees(transaction) {
const { state, domNode, input, options } = transaction;
const { state, mount, input, config: options } = transaction;
const { previousMarkup } = state;
const { inner } = options;
const domNodeAsHTMLEl = /** @type {HTMLElement} */ (domNode);
const { outerHTML } = domNodeAsHTMLEl;
const mountAsHTMLEl = /** @type {HTMLElement} */ (mount);
const { outerHTML } = mountAsHTMLEl;

// We rebuild the tree whenever the DOM Node changes, including the first
// time we patch a DOM Node.
if (previousMarkup !== outerHTML || !state.oldTree || !outerHTML) {
release(domNode);
state.oldTree = createTree(domNodeAsHTMLEl);
release(mount);
state.oldTree = createTree(mountAsHTMLEl);
protectVTree(state.oldTree);

// Reset the state cache after releasing.
StateCache.set(domNode, state);
StateCache.set(mount, state);
}

const { nodeName, attributes } = state.oldTree;
Expand Down
22 changes: 15 additions & 7 deletions packages/diffhtml/lib/tasks/schedule.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,23 +18,31 @@ export default function schedule(transaction) {

// Loop through all existing mounts to ensure we properly wait.
StateCache.forEach(val => {
// Is parent.
const domNode = /** @type {HTMLElement} */ (
val.activeTransaction && val.activeTransaction.domNode
const oldMount = /** @type {HTMLElement} */ (
val.activeTransaction && val.activeTransaction.mount
);
const newNode = /** @type {HTMLElement} */ (transaction.domNode);
const newMount = /** @type {HTMLElement} */ (transaction.mount);

if (!domNode || !domNode.contains || !newNode || !newNode.contains) {
// Only consider transactions that have mounts and are rendering.
if (!oldMount || !newMount || !val.isRendering) {
return;
}

// If the new mount point exists within an existing point that is rendering,
// then wait for that transaction to finish.
else if (
(domNode.contains(newNode) || newNode.contains(domNode)) &&
val.isRendering
oldMount.contains && oldMount.contains(newMount) ||
newMount.contains && newMount.contains(oldMount)
) {
state = val;
isRendering = true;
}
// Test if the active transaction is the same as the incoming by looking at
// the mount. Then look and see if the state is rendering.
else if (oldMount === newMount) {
state = val;
isRendering = true;
}
});

const { activeTransaction, nextTransaction } = state;
Expand Down
6 changes: 3 additions & 3 deletions packages/diffhtml/lib/tasks/should-update.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ import Transaction from "../transaction";
* @param {Transaction} transaction
*/
export default function shouldUpdate(transaction) {
const { domNode, input, state, state: { measure }, options } = transaction;
const { mount, input, state, state: { measure }, config: options } = transaction;
const prop = options.inner ? 'innerHTML' : 'outerHTML';

measure('should update');

const domNodeAsEl = /** @type {HTMLElement} */ (domNode);
const mountAsHTMLEl = /** @type {HTMLElement} */ (mount);

// If the contents haven't changed, abort the flow. Only support this if
// the new markup is a string, otherwise it's possible for our object
// recycling to match twice.
if (typeof input === 'string' && domNodeAsEl[prop]=== input) {
if (typeof input === 'string' && mountAsHTMLEl[prop]=== input) {
return transaction.abort(true);
}
else if (typeof input === 'string') {
Expand Down
8 changes: 4 additions & 4 deletions packages/diffhtml/lib/tasks/sync-trees.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import process from '../util/process';
import Transaction from '../transaction';

export default function syncTrees(/** @type {Transaction} */ transaction) {
const { state, state: { measure }, oldTree, newTree, domNode } = transaction;
const { state, state: { measure }, oldTree, newTree, mount } = transaction;

measure('sync trees');

Expand All @@ -31,7 +31,7 @@ export default function syncTrees(/** @type {Transaction} */ transaction) {
// If there is no `parentNode` for the replace operation, we will need to
// throw an error and prevent the `StateCache` from being updated.
if (process.env.NODE_ENV !== 'production') {
if (!/** @type {HTMLElement} */ (domNode).parentNode) {
if (!/** @type {HTMLElement} */ (mount).parentNode) {
throw new Error('Unable to replace top level node without a parent');
}
}
Expand All @@ -49,10 +49,10 @@ export default function syncTrees(/** @type {Transaction} */ transaction) {
const newNode = createNode(newTree);

// Update the StateCache since we are changing the top level element.
StateCache.delete(domNode);
StateCache.delete(mount);
StateCache.set(/** @type {Mount} */ (newNode), state);

transaction.domNode = /** @type {HTMLElement} */ (newNode);
transaction.mount = /** @type {HTMLElement} */ (newNode);

if (newTree.nodeName === 'script') {
state.scriptsToExecute.set(newTree, newTree.attributes.type || EMPTY.STR);
Expand Down
27 changes: 13 additions & 14 deletions packages/diffhtml/lib/transaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ export const tasks = {
export default class Transaction {
/**
*
* @param {Mount} domNode
* @param {Mount} mount
* @param {ValidInput} input
* @param {TransactionConfig} options
*/
static create(domNode, input, options) {
return new Transaction(domNode, input, options);
static create(mount, input, options) {
return new Transaction(mount, input, options);
}

/**
Expand Down Expand Up @@ -80,7 +80,7 @@ export default class Transaction {
*/
static assert(transaction) {
if (process.env.NODE_ENV !== 'production') {
if (typeof transaction.domNode !== 'object' || !transaction.domNode) {
if (typeof transaction.mount !== 'object' || !transaction.mount) {
throw new Error('Transaction requires a DOM Node mount point');
}

Expand Down Expand Up @@ -125,16 +125,15 @@ export default class Transaction {
* @param {TransactionConfig} config
*/
constructor(mount, input, config) {
// TODO: Rename this to mount.
this.domNode = mount;
this.mount = mount;
this.input = input;
// TODO: Rename this to config.
this.options = config;
this.config = config;

this.state = StateCache.get(mount) || /** @type {TransactionState} */ ({
measure: makeMeasure(mount, input),
measure: makeMeasure(this),
svgElements: new Set(),
scriptsToExecute: new Map(),
activeTransaction: this,
});

this.tasks = /** @type {Function[]} */ (
Expand Down Expand Up @@ -195,9 +194,9 @@ export default class Transaction {
* @return {Transaction}
*/
end() {
const { state, domNode, options } = this;
const { state, mount, config: options } = this;
const { measure, svgElements, scriptsToExecute } = state;
const domNodeAsHTMLEl = /** @type {HTMLElement} */ (domNode);
const mountAsHTMLEl = /** @type {HTMLElement} */ (mount);

measure('finalize');

Expand All @@ -218,7 +217,7 @@ export default class Transaction {
});

// Save the markup immediately after patching.
state.previousMarkup = 'outerHTML' in domNodeAsHTMLEl ? domNodeAsHTMLEl.outerHTML : EMPTY.STR;
state.previousMarkup = 'outerHTML' in mountAsHTMLEl ? mountAsHTMLEl.outerHTML : EMPTY.STR;

// Only execute scripts if the configuration is set. By default this is set
// to true. You can toggle this behavior for your app to disable script
Expand Down Expand Up @@ -280,10 +279,10 @@ export default class Transaction {
state = EMPTY.OBJ;

/** @type {Mount} */
domNode = EMPTY.STR;
mount = EMPTY.OBJ;

/** @type {ValidInput} */
input = EMPTY.STR;
input = EMPTY.OBJ;

/** @type {VTree=} */
oldTree = undefined;
Expand Down
47 changes: 26 additions & 21 deletions packages/diffhtml/lib/util/make-measure.js
Original file line number Diff line number Diff line change
@@ -1,50 +1,55 @@
import { Mount, ValidInput, VTree } from "./types";
import getConfig from './config';

export const marks = new Map();
export const prefix = 'diffHTML';
import getConfig from "./config";
import { VTree } from "./types";

const prefix = 'diffHTML';
const marks = new Map();
const nop = () => {};
let count = 0;

/**
* Creates a measure function that will collect data about the currently running
* transaction.
*
* @param {Mount} mount
* @param {ValidInput=} input
* @param {import('../transaction').default} transaction
* @return {(name: string) => void}
*/
export default function makeMeasure(mount, input) {
const wantsPerfChecks = getConfig('collectMetrics', false);

// If the user has not requested they want perf checks, return a nop
// function.
if (!wantsPerfChecks) { return nop; }

export default function makeMeasure(transaction) {
const { mount, input } = transaction;
const inputAsVTree = /** @type {VTree} */ (input);
const id = count++;

// Marks will only be available if the user has requested they want to collect
// metrics.
if (!getConfig('collectMetrics', false)) { return nop; }

return name => {
const host = /** @type any */ (mount).host;
name = `[${id}] ${name}`;

const { host } = /** @type {any} */ (mount);

// Use the Web Component name if it's available.
if (mount && host) {
name = `${host.constructor.name} ${name}`;
}
// Otherwise try an find the function name used.
else if (inputAsVTree && typeof inputAsVTree.rawNodeName === 'function') {
name = `${inputAsVTree.rawNodeName.name} ${name}`;
}

const endName = `${name}-end`;

if (!marks.has(name)) {
marks.set(name, performance.now());
performance.mark(name);
}
else {
const totalMs = (performance.now() - marks.get(name)).toFixed(3);
if (marks.has(name)) {
const prevMark = marks.get(name) || 0;
const totalMs = (performance.now() - prevMark).toFixed(3);

marks.delete(name);

performance.mark(endName);
performance.measure(`${prefix} ${name} (${totalMs}ms)`, name, endName);
}
else {
marks.set(name, performance.now());
performance.mark(name);
}
};
}
4 changes: 2 additions & 2 deletions packages/diffhtml/lib/util/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,8 @@ export const Supplemental = EMPTY.OBJ;
* @property {VTree=} oldTree
* @property {Boolean=} isRendering
* @property {String=} previousMarkup
* @property {any=} activeTransaction
* @property {any=} nextTransaction
* @property {import('../transaction').default} activeTransaction
* @property {import('../transaction').default=} nextTransaction
* @property {Document=} ownerDocument
*/
export const TransactionState = EMPTY.OBJ;
Expand Down
Loading