Skip to content

Commit

Permalink
Add unit tests and improve config
Browse files Browse the repository at this point in the history
  • Loading branch information
@tbranyen
tbranyen committed Jun 4, 2021
1 parent e0bf87a commit 74a750d
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 66 deletions.
68 changes: 19 additions & 49 deletions packages/diffhtml/lib/node/patch.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ const blocklist = new Set();
const allowlist = new Set();

/**
* Directly set this attributes, in addition to setting as properties.
* @type {any}
* Directly set these attributes in addition to setting as properties.
* @type {string[]}
*/
const DIRECT = ['class', 'checked', 'disabled', 'selected'];

Expand Down Expand Up @@ -69,9 +69,7 @@ const setAttribute = (vTree, domNode, name, value) => {
const noValue = value === null || value === undefined;

// If we cannot set the value as a property, try as an attribute.
if (lowerName) {
htmlElement.setAttribute(lowerName, noValue ? EMPTY.STR : value);
}
htmlElement.setAttribute(lowerName, noValue ? EMPTY.STR : value);
}
// Support patching an object representation of the style object.
else if (isObject && lowerName === 'style') {
Expand All @@ -97,28 +95,22 @@ const removeAttribute = (domNode, name) => {

// Runtime checking if the property can be set.
const blocklistName = /** @type {HTMLElement} */ (domNode).nodeName + '-' + name;
const anyNode = /** @type {any} */ (domNode);

if (allowlist.has(blocklistName)) {
const anyNode = /** @type {any} */ (domNode);
if (isEvent) {
anyNode[name] = undefined;
}

if (isEvent) {
anyNode[name] = undefined;
} else {
delete anyNode[name];
}
if (allowlist.has(blocklistName)) {
delete anyNode[name];

if (DIRECT.includes(name)) {
/** @type {any} */ (domNode)[name] = false;
}
}
else if (!blocklist.has(blocklistName)) {
try {
const anyNode = /** @type {any} */ (domNode);
if (isEvent) {
anyNode[name] = undefined;
} else {
delete anyNode[name];
}
delete anyNode[name];

if (DIRECT.includes(name)) {
/** @type {any} */ (domNode)[name] = false;
Expand Down Expand Up @@ -184,10 +176,6 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
createNode(vTree, ownerDocument, isSVG)
);

if (!domNode) {
break;
}

const oldValue = domNode.getAttribute(name);
const attributeChangedPromises = runTransitions(
'attributeChanged', vTree, name, oldValue, value
Expand Down Expand Up @@ -219,10 +207,6 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
createNode(vTree, ownerDocument, isSVG)
);

if (!domNode) {
break;
}

const oldValue = domNode.getAttribute(name);
const attributeChangedPromises = runTransitions(
'attributeChanged', vTree, name, oldValue, null
Expand Down Expand Up @@ -255,10 +239,6 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
createNode(vTree, ownerDocument, isSVG)
);

if (!domNode) {
break;
}

protectVTree(vTree);

const textChangedPromises = runTransitions(
Expand All @@ -285,18 +265,16 @@ export default function patchNode(patches, state = EMPTY.OBJ) {

i += 4;

if (!NodeCache.has(vTree)) {
continue;
}

// First attempt to locate a pre-existing DOM Node. If one hasn't been
// created there could be a few reasons.
let domNode = /** @type {HTMLElement} */ (
NodeCache.get(vTree)
);

// Patching without an existing DOM Node is a mistake, so we should not
// attempt to do anything in this case.
if (!domNode) {
break;
}

const isSVG = svgElements.has(newTree);

protectVTree(newTree);
Expand Down Expand Up @@ -352,18 +330,14 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
// the constraints. If there are no transitions, do not use the more
// expensive, but more expressive path.
if (!hasAttached && !hasDetached && !hasReplaced) {
if (oldDomNode.parentNode) {
oldDomNode.parentNode.replaceChild(newDomNode, oldDomNode);
unprotectVTree(oldTree);
}
oldDomNode.parentNode.replaceChild(newDomNode, oldDomNode);
unprotectVTree(oldTree);

break;
}

// Always insert before to allow the element to transition.
if (oldDomNode.parentNode) {
oldDomNode.parentNode.insertBefore(newDomNode, oldDomNode);
}
oldDomNode.parentNode.insertBefore(newDomNode, oldDomNode);

const allPromises = [
...(hasAttached && runTransitions('attached', newTree) || EMPTY.ARR),
Expand All @@ -383,9 +357,7 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
promises.push(...allPromises);
}
else {
if (oldDomNode.parentNode) {
oldDomNode.parentNode.removeChild(oldDomNode);
}
oldDomNode.parentNode.removeChild(oldDomNode);
unprotectVTree(oldTree);
}

Expand Down Expand Up @@ -416,9 +388,7 @@ export default function patchNode(patches, state = EMPTY.OBJ) {
promises.push(...detachedPromises);
}
else {
if (domNode.parentNode) {
domNode.parentNode.removeChild(domNode);
}
domNode.parentNode.removeChild(domNode);
unprotectVTree(vTree);
}

Expand Down
5 changes: 3 additions & 2 deletions packages/diffhtml/lib/tree/create.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ export default function createTree(input, attributes, childNodes, ...rest) {

// When using an Array copy the Nodes in and ensure a valid top-level tree.
for (let i = 0; i < length; i++) {
if (!input) continue;
childNodes.push(input[i]);
const hasInput = input && !input[i];
if (hasInput) continue;
input && childNodes.push(input[i]);
}

entry = createTree(fragmentName, null, childNodes);
Expand Down
29 changes: 14 additions & 15 deletions packages/diffhtml/lib/util/config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ import globalThis from './global';

const { parseInt } = Number;
const { parse } = JSON;
const { location, URLSearchParams } = globalThis;
const hasSearchParams = typeof URLSearchParams !== 'undefined';
const hasLocation = typeof location !== 'undefined';
const useSearchParams = hasSearchParams && hasLocation;
const useEnv = process.env;

/** @type {Config} */
export const globalConfig = {
Expand Down Expand Up @@ -40,10 +35,6 @@ function formatValue(value, type) {
return parseInt(valueAsString, 10);
}

case 'array': {
return valueAsString.split(',');
}

case 'object': {
return parse(valueAsString);
}
Expand All @@ -64,6 +55,12 @@ function formatValue(value, type) {
* @return {unknown}
*/
export default function getConfig(name, defaultValue, type = typeof defaultValue, overrides) {
const { location, URLSearchParams } = globalThis;
const hasSearchParams = typeof URLSearchParams !== 'undefined';
const hasLocation = typeof location !== 'undefined';
const useSearchParams = hasSearchParams && hasLocation;
const useEnv = process.env;

// Allow bypassing any lookups if overrides are passed and match the config
// being looked up.
if (overrides && name in overrides) {
Expand All @@ -72,22 +69,24 @@ export default function getConfig(name, defaultValue, type = typeof defaultValue

// The keyname for lookups via search params or env variable is DIFF_key and
// is case-insensitive. This is why we lowercaes the entire lookup.
const keyName = `DIFF_${name.replace(/[^a-zA-Z0-9]/, '')}`.toLowerCase();
const keyName = `DIFF_${name.replace(/[^a-zA-Z0-9]/, '')}`;

// Try URL search params first.
if (useSearchParams) {
const searchParams = new URLSearchParams(location.search);
const lowerKey = keyName.toLowerCase();

// Use has here, because boolean values can be set with only a key.
if (searchParams.has(keyName)) {
return formatValue(decodeURIComponent(String(searchParams.get(keyName))), type);
if (searchParams.has(lowerKey)) {
return formatValue(decodeURIComponent(String(searchParams.get(lowerKey))), type);
}
}

// Try environment variables.
if (useEnv && keyName in process.env) {
return formatValue(process.env[keyName], type);
const upperKey = keyName.toUpperCase();
if (useEnv && upperKey in process.env) {
return formatValue(process.env[upperKey.toUpperCase()], type);
}

return defaultValue;
}
}
14 changes: 14 additions & 0 deletions packages/diffhtml/test/tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,20 @@ describe('Tree', function() {
});
});

it('will ignore falsy values when creating fragment from an array', () => {
const vTree = createTree([createTree('div'), null, createTree('p')]);

deepStrictEqual(vTree, {
rawNodeName: '#document-fragment',
nodeName: '#document-fragment',
nodeValue: '',
nodeType: 11,
key: '',
attributes: {},
childNodes: [createTree('div'), createTree('p')],
});
});

it('will ignore falsy values when creating text from an array', () => {
const vTree = createTree('#text', ['some text', null, ' ', 'chunks']);

Expand Down
66 changes: 66 additions & 0 deletions packages/diffhtml/test/util.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import _process from '../lib/util/process';
import { protectVTree, unprotectVTree, gc } from '../lib/util/memory';
import makeMeasure from '../lib/util/make-measure';
import Pool from '../lib/util/pool';
import getConfig from '../lib/util/config';
import validateMemory from './util/validate-memory';
import createSupplemental from './util/create-supplemental';

const { floor } = Math;
const { stringify } = JSON;

describe('Util', function() {
let performance;
Expand All @@ -34,7 +36,71 @@ describe('Util', function() {
});

describe('Config', () => {
it('will load boolean type from process.env', () => {
process.env.DIFF_TESTBOOL = 'true';

const actual = getConfig('test_bool', false);
strictEqual(actual, true);
});

it('will load boolean type from search params', () => {
location.search = '?diff_testbool=true';

const actual = getConfig('test_bool', false);
strictEqual(actual, true);
});

it('will load string type from process.env', () => {
process.env.DIFF_TESTSTR = 'some-str';

const actual = getConfig('test_str', '');
strictEqual(actual, 'some-str');
});

it('will load string type from search params', () => {
location.search = '?diff_teststr=test-str';

const actual = getConfig('test_str', '');
strictEqual(actual, 'some-str');
});

it('will load number type from process.env', () => {
process.env.DIFF_TESTNUM = '144';

const actual = getConfig('test_num', -1);
strictEqual(actual, 144);
});

it('will load number type from search params', () => {
location.search = '?diff_testnum=144';

const actual = getConfig('test_num', -1);
strictEqual(actual, 144);
});

it('will load object type from process.env', () => {
process.env.DIFF_TESTOBJ = stringify({ 1: true, 2: false });

const actual = getConfig('test_obj', {});
deepStrictEqual(actual, { 1: true, 2: false });
});

it('will load object type from search params', () => {
location.search = `?diff_testobj=${stringify({ 1: true, 2: false })}`;

const actual = getConfig('test_obj', {});
deepStrictEqual(actual, { 1: true, 2: false });
});

it('will not error if not in a browser env', () => {
const oldLocation = global.location;

delete global.location;

strictEqual(getConfig('t', 1), 1);

global.location = oldLocation;
});
});

describe('DecodeEntities', () => {
Expand Down

0 comments on commit 74a750d

Please sign in to comment.