Skip to content

Commit

Permalink
Adding a couple minor changes for 2.x interface.
Browse files Browse the repository at this point in the history
Changes:
* Stricter handling of “textarea” binding. Previously we were overly
  lenient in how the binding worked which caused automagical behavior.
  For example, newlines were quietly ignored, which could lead to issues
  if you really _wanted_ newlines in a default value in a text area
  control and were actually trying to interpolate between lines. Now, it
  just throws for that case.
* Spec-compliant attribute traversal. Previously, we relied on the
  browser convention that attributes in a NamedNodeMap would be iterated
  over in a particular manner. The spec strictly indicates that this is
  not to be relied on — so we can oblige.
* The deprecated `plaintext` html tag is no longer considered. Use at
  your own peril…
* General improvements to inline documentation and naming of things.
  • Loading branch information
theengineear committed Nov 16, 2024
1 parent 73548db commit 8b049e1
Show file tree
Hide file tree
Showing 3 changed files with 197 additions and 23 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- The `ifDefined` updater now deletes the attribute on `null` in addition to
`undefined`. This makes it behave identically to `nullish`. However, both
updaters are deprecated and the `??attr` binding should be used instead.
- Interpolation of `textarea` is stricter. This used to be handled with some
leniency — `<textarea>\n ${value} \n</textarea>`. Now, you have to fit the
interpolation exactly — `<textarea></textarea>`.

### Deprecated

- The `ifDefined` and `nullish` updaters are deprecated, update templates to use
syntax like `??foo="${bar}"`.
- The `repeat` updater is deprecated, use `map` instead.
- The `unsafeHTML` and `unsafeSVG` updaters are deprecated, use `unsafe`.
- The `plaintext` tag is no longer handled. This is a deprecated html tag which
required special handling… but it’s unlikely that anyone is using that.

### Fixed

Expand Down
101 changes: 101 additions & 0 deletions test/test-template-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,75 @@ describe('html rendering', () => {
container.remove();
});

// Unlike a NodeList, a NamedNodeMap (i.e., “.attributes”) is not technically
// ordered in any way. This test just confirms that the template engine logic
// doesn’t get confused in any way post-parse.
it('renders elements with many attributes in a weird order', () => {
const getTemplate = ({
property1,
z99,
defined,
dataFoo,
property2,
title,
dataBar,
className,
property3,
boolean,
ariaLabel,
content,
}) => {
return html`
<div
id="target"
.property1="${property1}"
z-99="${z99}"
??defined="${defined}"
data-foo="${dataFoo}"
.property2="${property2}"
title="${title}"
static="just hanging"
data-bar="${dataBar}"
class="${className}"
.property3="${property3}"
?boolean="${boolean}"
aria-label="${ariaLabel}">
${content}
</div>
`;
};
const container = document.createElement('div');
document.body.append(container);
render(container, getTemplate({
property1: null,
z99: true,
defined: false,
dataFoo: 10,
property2: -1,
title: 'a title',
dataBar: 'data attribute',
className: 'a b c',
property3: new URL('https://github.com/Netflix/x-element'),
boolean: 'yes',
ariaLabel: 'this is what it does',
content: 'influencing',
}));
const target = container.querySelector('#target');
assert(target.property1 === null);
assert(target.getAttribute('z-99') === 'true');
assert(target.getAttribute('defined') === 'false');
assert(target.getAttribute('data-foo') === '10');
assert(target.property2 === -1);
assert(target.getAttribute('title') === 'a title');
assert(target.getAttribute('data-bar') === 'data attribute');
assert(target.getAttribute('class') === 'a b c');
assert(target.property3.href === 'https://github.com/Netflix/x-element');
assert(target.getAttribute('boolean') === '');
assert(target.getAttribute('aria-label') === 'this is what it does');
assert(target.textContent.trim() === 'influencing');
container.remove();
});

it('renders multiple, interpolated content', () => {
const getTemplate = ({ one, two }) => {
return html`
Expand Down Expand Up @@ -1058,6 +1127,38 @@ describe('rendering errors', () => {
container.remove();
});

it('throws when attempting non-trivial interpolation of a textarea tag', () => {
const getTemplate = ({ content }) => {
return html`<textarea id="target">please ${content} no</textarea>`;
};
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, getTemplate({ content: 'foo' }));
} catch (e) {
error = e;
}
assert(error?.message === `Only basic interpolation of "textarea" tags is allowed.`, error.message);
container.remove();
});

it('throws when attempting non-trivial interpolation of a title tag', () => {
const getTemplate = ({ content }) => {
return html`<title id="target">please ${content} no</title>`;
};
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, getTemplate({ defaultValue: 'foo' }));
} catch (e) {
error = e;
}
assert(error?.message === `Only basic interpolation of "title" tags is allowed.`, error.message);
container.remove();
});

it('throws for unquoted attributes', () => {
const templateResultReference = html`<div id="target" not-ok=${'foo'}>Gotta double-quote those.</div>`;
const container = document.createElement('div');
Expand Down
114 changes: 91 additions & 23 deletions x-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -1018,6 +1018,7 @@ class TemplateEngine {
static #DEFINED_PREFIX = 'x-element-defined';
static #PROPERTY_PREFIX = 'x-element-property';
static #CONTENT_PREFIX = 'x-element-content';
static #ATTRIBUTE_PADDING = 6;

// Patterns to find special edges in original html strings.
static #OPEN_REGEX = /<[a-z][a-z0-9-]*(?=\s)/g;
Expand Down Expand Up @@ -1475,6 +1476,9 @@ class TemplateEngine {
TemplateEngine.#mapInner(node, startNode, identify, callback, value, 'repeat');
}

// Walk through each string from our tagged template function “strings” array
// in a stateful way so that we know what kind of bindings are implied at
// each interpolated value.
static #exhaustString(string, state) {
if (!state.inside) {
// We're outside the opening tag.
Expand Down Expand Up @@ -1502,6 +1506,29 @@ class TemplateEngine {
}
}

// Flesh out an html string from our tagged template function “strings” array
// and add special markers that we can detect later, after instantiation.
//
// E.g., the user might have passed this interpolation:
//
// <div
// id="foo-bar-baz"
// foo="${foo}"
// bar="${bar}"
// .baz="${baz}">
// ${content}
// </div>
//
// … and we would instrument it as follows:
//
// <div
// id="foo-bar-baz"
// x-element-attribute-000001="foo"
// x-element-attribute-000002="bar"
// x-element-property-000003="baz">
// <!--x-element-content-->
// </div>
//
static #createHtml(type, strings) {
const htmlStrings = [];
const state = { inside: false, index: 0 };
Expand All @@ -1527,13 +1554,15 @@ class TemplateEngine {
case '??': prefix = TemplateEngine.#DEFINED_PREFIX; syntax = 4; break;
case '?': prefix = TemplateEngine.#BOOLEAN_PREFIX; syntax = 3; break;
}
string = string.slice(0, -syntax - attribute.length) + `${prefix}-${iii}="${attribute}`;
const index = String(iii).padStart(TemplateEngine.#ATTRIBUTE_PADDING, '0');
string = string.slice(0, -syntax - attribute.length) + `${prefix}-${index}="${attribute}`;
} else {
// We found a match like this: html`<div .title="${value}"></div>`.
// The syntax takes up 3 characters: `.${property}="`.
const syntax = 3;
const prefix = TemplateEngine.#PROPERTY_PREFIX;
string = string.slice(0, -syntax - property.length) + `${prefix}-${iii}="${property}`;
const index = String(iii).padStart(TemplateEngine.#ATTRIBUTE_PADDING, '0');
string = string.slice(0, -syntax - property.length) + `${prefix}-${index}="${property}`;
}
state.index = 1; // Accounts for an expected quote character next.
} else {
Expand Down Expand Up @@ -1563,8 +1592,35 @@ class TemplateEngine {
return template.content;
}

static #findLookups(node, nodeType = Node.DOCUMENT_FRAGMENT_NODE, path = []) {
const lookups = [];
// Walk through our fragment that we added special markers to and collect
// paths to each future target. We use “paths” because each future instance
// will clone this fragment and so paths are all we can really cache. And,
// while we go through, clean up our bespoke markers.
// Note that we are always walking the interpolated strings and the resulting,
// instantiated DOM _in the same depth-first manner_. This means that the
// ordering is fairly reliable. The only special handling we need to do is to
// ensure that we don’t rely on the ordering of NamedNodeMap objects since
// the spec doesn’t guarantee anything there (though in practice, it would
// probably work…).
//
// For example, we walk this structure:
//
// <div
// id="foo-bar-baz"
// x-element-attribute-000001="foo"
// x-element-attribute-000002="bar"
// x-element-property-000003="baz">
// <!--x-element-content-->
// </div>
//
// And end up with this (which is ready to be injected into a container):
//
// <div id="foo-bar-baz">
// <!---->
// <!---->
// </div>
//
static #findLookups(node, nodeType = Node.DOCUMENT_FRAGMENT_NODE, lookups = [], path = []) {
// @ts-ignore — TypeScript doesn’t seem to understand the nodeType param.
if (nodeType === Node.ELEMENT_NODE) {
// Copy the live NamedNodeMap since we need to mutate it during iteration.
Expand All @@ -1581,8 +1637,9 @@ class TemplateEngine {
? 'defined'
: null;
if (type) {
const index = Number(name.slice(-TemplateEngine.#ATTRIBUTE_PADDING));
const value = attribute.value;
lookups.push({ path, type, name: value });
lookups[index] = { path, type, name: value };
node.removeAttribute(name);
}
}
Expand All @@ -1593,13 +1650,14 @@ class TemplateEngine {
node.textContent.includes(TemplateEngine.#CONTENT_PREFIX)
) {
throw new Error(`Interpolation of "${localName}" tags is not allowed.`);
} else if (
localName === 'plaintext' ||
localName === 'textarea' ||
localName === 'title'
) {
} else if (localName === 'textarea' || localName === 'title') {
if (node.textContent.includes(TemplateEngine.#CONTENT_PREFIX)) {
lookups.push({ path, type: 'text' });
if (node.textContent === `<!--${TemplateEngine.#CONTENT_PREFIX}-->`) {
node.textContent = '';
lookups.push({ path, type: 'text' });
} else {
throw new Error(`Only basic interpolation of "${localName}" tags is allowed.`);
}
}
}
} else if (
Expand All @@ -1621,13 +1679,18 @@ class TemplateEngine {
for (const childNode of node.childNodes) {
const childNodeType = childNode.nodeType;
if (childNodeType === Node.ELEMENT_NODE || Node.COMMENT_NODE) {
lookups.push(...TemplateEngine.#findLookups(childNode, childNodeType, [...path, iii++]));
TemplateEngine.#findLookups(childNode, childNodeType, lookups, [...path, iii++]);
}
}
}
return lookups;
if (nodeType === Node.DOCUMENT_FRAGMENT_NODE) {
return lookups;
}
}

// After cloning our common fragment, we use the “lookups” to cache live
// references to DOM nodes so that we can surgically perform updates later in
// an efficient manner. Lookups are like directions to find our real targets.
static #findTargets(fragment, lookups) {
const targets = [];
const cache = new Map();
Expand Down Expand Up @@ -1659,6 +1722,7 @@ class TemplateEngine {
return targets;
}

// Create and prepare a document fragment to be injected into some container.
static #ready(result) {
if (result.readied) {
throw new Error(`Unexpected re-injection of template result.`);
Expand All @@ -1678,16 +1742,6 @@ class TemplateEngine {
Object.assign(result, { fragment, entries });
}

static #inject(result, node, options) {
const nodes = result.type === 'svg'
? result.fragment.firstChild.childNodes
: result.fragment.childNodes;
options?.before
? TemplateEngine.#insertAllBefore(node.parentNode, node, nodes)
: TemplateEngine.#insertAllBefore(node, null, nodes);
result.fragment = null;
}

static #assign(result, newResult) {
result.lastValues = result.values;
result.values = newResult.values;
Expand Down Expand Up @@ -1845,6 +1899,8 @@ class TemplateEngine {
}
}

// Bind the current values from a result by walking through each target and
// updating the DOM if things have changed.
static #commit(result) {
result.lastValues ??= result.values.map(() => TemplateEngine.#UNSET);
const { entries, values, lastValues } = result;
Expand All @@ -1862,6 +1918,18 @@ class TemplateEngine {
}
}

// Attach a document fragment into some container. Note that all the DOM in
// the fragment will already have values correctly bound.
static #inject(result, node, options) {
const nodes = result.type === 'svg'
? result.fragment.firstChild.childNodes
: result.fragment.childNodes;
options?.before
? TemplateEngine.#insertAllBefore(node.parentNode, node, nodes)
: TemplateEngine.#insertAllBefore(node, null, nodes);
result.fragment = null;
}

static #throwUpdaterError(updater, type) {
switch (updater) {
case TemplateEngine.#live:
Expand Down

0 comments on commit 8b049e1

Please sign in to comment.