From bc2305568fec1c14a4881a9ba2d07943f710fd6b Mon Sep 17 00:00:00 2001 From: Andrew Seier Date: Mon, 18 Nov 2024 15:58:11 -0800 Subject: [PATCH] =?UTF-8?q?Restrict=20=E2=80=9Cunsafe=E2=80=9D=20usage=20f?= =?UTF-8?q?or=20initial=20=E2=80=9C2.x=E2=80=9D=20release.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changes: * So far, we only use “unsafe” for html — so we just keep that for now. * Stopped assuming that the “template engine” discussed in the “CHANGELOG.md” is the “default template engine” — i.e., we explicitly write that now. * Simplified formatting related to bindings in “TEMPLATES.md”. --- CHANGELOG.md | 48 ++++--- doc/TEMPLATES.md | 56 ++------ test/test-template-engine.js | 59 ++------ x-element.js | 261 +++++++++++++++++------------------ 4 files changed, 172 insertions(+), 252 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1877ad..e9c9707 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,42 +8,48 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- You can now bind attributes with `??foo="${bar}"` syntax. This is functionally - equivalent to the `nullish` updater and will replace that functionality later. -- A new `unsafe` updater was added to replace `unsafeHTML` and `unsafeSVG`. You - use it like `unsafe(value, 'html')` and `unsafe(value, 'svg')`. +- You can now bind attributes with `??foo="${bar}"` syntax in the default + template engine. This is functionally equivalent to the `nullish` updater from + the default template engine and will replace that functionality later. +- A new `unsafe` updater was added to replace `unsafeHTML` in the default + template engine. You use it like `unsafe(value)`. Only unsafe html injection + will be supported in the default template engine. ### Changed - Template errors now include approximate line numbers from the offending - template. They also print the registered custom element tag name (#201). + template in the default template engine. They also print the registered custom + element tag name (#201). - 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 — ``. Now, you have to fit the - interpolation exactly — ``. + `undefined` in the default template engine. This makes it behave identically + to `nullish` in the default template engine. However, both updaters are + deprecated — the `??attr` binding should be used instead when using the + default template engine (#204). +- Interpolation of `textarea` is more strict in the default template engine. + This used to be handled with some leniency for newlines in templates — + ``. Now, you have to interpolate exactly — + ``. ### 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 `repeat` updater is deprecated, use `map` instead (#204). +- The `unsafeHTML` and `unsafeSVG` updaters are deprecated, use `unsafe` (#204). - 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 -- Transitions from different content values should all now work. For example, - you previously could not change from a text value to an array. Additionally, - state is properly cleared when going from one value type to another — e.g., - when going from `unsafe` back to `null`. -- The `map` updater throws immediately when given non-array input. Previously, - it only threw _just before_ it was bound as content. -- Dummy content cursor is no longer appended to end of template. This was an - innocuous off-by-one error when creating instrumented html from the tagged - template strings. +- Transitions from different content values should all now work for the default + template engine. For example, you previously could not change from a text + value to an array. Additionally, state is properly cleared when going from one + value type to another — e.g., when going from `unsafe` back to `null`. +- The `map` updater throws immediately when given non-array input for the + default template engine. Previously, it only threw _just before_ it was bound. +- Dummy content cursor is no longer appended to end of template for the default + template engine. This was an innocuous off-by-one error when creating + instrumented html from the tagged template strings. ## [1.1.1] - 2024-11-09 diff --git a/doc/TEMPLATES.md b/doc/TEMPLATES.md index 67fab91..77947ce 100644 --- a/doc/TEMPLATES.md +++ b/doc/TEMPLATES.md @@ -21,49 +21,19 @@ static template(html, { map }) { } ``` -The following binding types are supported: - -| Type | Example | -| :------------------ | :----------------------------------------- | -| attribute | `` | -| attribute (boolean) | `` | -| attribute (defined) | `` | -| property | `` | -| content | `${foo}` | - -Emulates: - -```javascript -const el = document.createElement('div'); -el.attachShadow({ mode: 'open' }); -el.innerHTML = ''; -const target = el.shadowRoot.getElementById('target'); - -// attribute value bindings set the attribute value -target.setAttribute('foo', bar); - -// attribute boolean bindings set the attribute to an empty string or remove -target.setAttribute('foo', ''); // when bar is truthy -target.removeAttribute('foo'); // when bar is falsy - -// attribute defined bindings set the attribute if the value is non-nullish -target.setAttribute('foo', bar); // when bar is non-nullish -target.removeAttribute('foo'); // when bar is nullish - -// property bindings assign the value to the property of the node -target.foo = bar; - -// content bindings create text nodes for basic content -const text = document.createTextNode(''); -text.textContent = foo; -target.append(text); - -// content bindings append a child for singular, nested content -target.append(foo); - -// content binding maps and appends children for arrays of nested content -target.append(...foo); -``` +The following bindings are supported: + +| Binding | Template | Emulates | +| :------------------ | :--------------------------- | :------------------------------------------------------------ | +| -- | -- | `const el = document.createElement('div');` | +| attribute | `
` | `el.setAttribute('foo', bar);` | +| attribute (boolean) | `
` | `el.setAttribute('foo', ''); // if “bar” is truthy` | +| -- | -- | `el.removeAttribute('foo'); // if “bar” is falsy` | +| attribute (defined) | `
` | `el.setAttribute('foo', bar); // if “bar” is non-nullish` | +| -- | -- | `el.removeAttribute('foo'); // if “bar” is nullish` | +| property | `
` | `el.foo = bar;` | +| content | `
${foo}
` | `el.append(document.createTextNode(foo)) // if “bar” is text` | +| -- | -- | (see [content binding](#content-binding) for composition) | **Important note on serialization during data binding:** diff --git a/test/test-template-engine.js b/test/test-template-engine.js index 2d89ff4..c797140 100644 --- a/test/test-template-engine.js +++ b/test/test-template-engine.js @@ -564,7 +564,7 @@ describe('html updaters', () => { it('unsafe html', () => { const getTemplate = ({ content }) => { - return html`
${unsafe(content, 'html')}
`; + return html`
${unsafe(content)}
`; }; const container = document.createElement('div'); document.body.append(container); @@ -892,7 +892,7 @@ describe('html updaters', () => { const resolve = (type, value) => { switch(type) { case 'map': return map(value, item => item.id, item => html`
`); - case 'html': return unsafe(value, 'html'); + case 'html': return unsafe(value); default: return value; // E.g., an array, some text, null, undefined, etc. } }; @@ -1032,31 +1032,6 @@ describe('svg rendering', () => { }); describe('svg updaters', () => { - it('unsafe svg', () => { - const getTemplate = ({ content }) => { - return html` - - ${unsafe(content, 'svg')} - - `; - }; - const container = document.createElement('div'); - document.body.append(container); - render(container, getTemplate({ content: '' })); - assert(!!container.querySelector('#injected')); - assert(container.querySelector('#injected').getBoundingClientRect().height = 20); - assert(container.querySelector('#injected').getBoundingClientRect().width = 20); - render(container, getTemplate({ content: '' })); - assert(!!container.querySelector('#injected')); - assert(container.querySelector('#injected').getBoundingClientRect().height = 10); - assert(container.querySelector('#injected').getBoundingClientRect().width = 10); - container.remove(); - }); - it('unsafeSVG', () => { const getTemplate = ({ content }) => { return html` @@ -1476,28 +1451,10 @@ describe('rendering errors', () => { describe('unsafe', () => { - it('throws if used on an unexpected language', () => { - const expected = 'Unexpected unsafe language "css". Expected "html" or "svg".'; - const getTemplate = ({ maybe }) => { - return html`
`; - }; - const container = document.createElement('div'); - document.body.append(container); - let actual; - try { - render(container, getTemplate({ maybe: 'yes' })); - } catch (error) { - actual = error.message; - } - assert(!!actual, 'No error was thrown.'); - assert(actual === expected, actual); - container.remove(); - }); - it('throws if used on an "attribute"', () => { const expected = 'The unsafe update must be used on content, not on an attribute.'; const getTemplate = ({ maybe }) => { - return html`
`; + return html`
`; }; const container = document.createElement('div'); document.body.append(container); @@ -1515,7 +1472,7 @@ describe('rendering errors', () => { it('throws if used on a "boolean"', () => { const expected = 'The unsafe update must be used on content, not on a boolean attribute.'; const getTemplate = ({ maybe }) => { - return html`
`; + return html`
`; }; const container = document.createElement('div'); document.body.append(container); @@ -1533,7 +1490,7 @@ describe('rendering errors', () => { it('throws if used on a "defined"', () => { const expected = 'The unsafe update must be used on content, not on a defined attribute.'; const getTemplate = ({ maybe }) => { - return html`
`; + return html`
`; }; const container = document.createElement('div'); document.body.append(container); @@ -1551,7 +1508,7 @@ describe('rendering errors', () => { it('throws if used with a "property"', () => { const expected = 'The unsafe update must be used on content, not on a property.'; const getTemplate = ({ maybe }) => { - return html`
`; + return html`
`; }; const container = document.createElement('div'); document.body.append(container); @@ -1569,7 +1526,7 @@ describe('rendering errors', () => { it('throws if used with "text"', () => { const expected = 'The unsafe update must be used on content, not on text content.'; const getTemplate = ({ maybe }) => { - return html``; + return html``; }; const container = document.createElement('div'); document.body.append(container); @@ -1588,7 +1545,7 @@ describe('rendering errors', () => { const getTemplate = ({ content }) => { return html`
- ${unsafe(content, 'html')} + ${unsafe(content)}
`; }; diff --git a/x-element.js b/x-element.js index bc5ff8a..32079d4 100644 --- a/x-element.js +++ b/x-element.js @@ -1122,13 +1122,10 @@ class TemplateEngine { const result = TemplateEngine.#symbolToResult.get(resultReference); if (TemplateEngine.#cannotReuseResult(state.result, result)) { TemplateEngine.#removeWithin(container); - TemplateEngine.#ready(result); - TemplateEngine.#commit(result); TemplateEngine.#inject(result, container); state.result = result; } else { - TemplateEngine.#assign(state.result, result); - TemplateEngine.#commit(state.result); + TemplateEngine.#update(state.result, result); } } else { TemplateEngine.#clearObject(state); @@ -1194,23 +1191,19 @@ class TemplateEngine { } /** - * Updater to inject trusted “html” or “svg” into the DOM. - * Use with caution. The "unsafe" updater allows arbitrary input to be - * parsed and injected into the DOM. + * Updater to inject trusted “html” into the DOM. + * Use with caution. The "unsafe" updater allows arbitrary input to be parsed + * and injected into the DOM. * ```js - * html`
${unsafe(obj.trustedMarkup, 'html')}
`; + * html`
${unsafe(obj.trustedMarkup)}
`; * ``` * @param {any} value - * @param {'html'|'svg'} language * @returns {any} */ - static unsafe(value, language) { - if (language !== 'html' && language !== 'svg') { - throw new Error(`Unexpected unsafe language "${language}". Expected "html" or "svg".`); - } + static unsafe(value) { const symbol = Object.create(null); const updater = TemplateEngine.#unsafe; - const update = { updater, value, language }; + const update = { updater, value }; TemplateEngine.#symbolToUpdate.set(symbol, update); return symbol; } @@ -1342,19 +1335,13 @@ class TemplateEngine { } } - static #unsafe(node, startNode, value, lastValue, language) { + static #unsafe(node, startNode, value, lastValue) { if (value !== lastValue) { if (typeof value === 'string') { const template = document.createElement('template'); - if (language === 'html') { - template.innerHTML = value; - TemplateEngine.#removeBetween(startNode, node); - TemplateEngine.#insertAllBefore(node.parentNode, node, template.content.childNodes); - } else { - template.innerHTML = `${value}`; - TemplateEngine.#removeBetween(startNode, node); - TemplateEngine.#insertAllBefore(node.parentNode, node, template.content.firstChild.childNodes); - } + template.innerHTML = value; + TemplateEngine.#removeBetween(startNode, node); + TemplateEngine.#insertAllBefore(node.parentNode, node, template.content.childNodes); } else { throw new Error(`Unexpected unsafe value "${value}".`); } @@ -1389,91 +1376,13 @@ class TemplateEngine { } } - static #mapInner(node, startNode, identify, callback, inputs, name) { - const state = TemplateEngine.#setIfMissing(TemplateEngine.#nodeToArrayState, startNode, () => ({})); - if (!state.map) { - TemplateEngine.#clearObject(state); - state.map = new Map(); - let index = 0; - for (const input of inputs) { - const reference = callback ? callback(input, index) : input; - const result = TemplateEngine.#symbolToResult.get(reference); - if (result) { - const id = identify ? identify(input, index) : String(index); - const cursors = TemplateEngine.#createCursors(node); - TemplateEngine.#ready(result); - TemplateEngine.#commit(result); - TemplateEngine.#inject(result, cursors.node, { before: true }); - state.map.set(id, { id, result, ...cursors }); - } else { - throw new Error(`Unexpected ${name} value "${reference}" provided by callback.`); - } - index++; - } - } else { - let lastItem; - const ids = new Set(); - let index = 0; - for (const input of inputs) { - const reference = callback ? callback(input, index) : input; - const result = TemplateEngine.#symbolToResult.get(reference); - if (result) { - const id = identify ? identify(input, index) : String(index); - if (state.map.has(id)) { - const item = state.map.get(id); - if (TemplateEngine.#cannotReuseResult(item.result, result)) { - // Add new comment cursors before removing old comment cursors. - const cursors = TemplateEngine.#createCursors(item.startNode); - TemplateEngine.#removeThrough(item.startNode, item.node); - TemplateEngine.#ready(result); - TemplateEngine.#commit(result); - TemplateEngine.#inject(result, cursors.node, { before: true }); - Object.assign(item, { result, ...cursors }); - } else { - TemplateEngine.#assign(item.result, result); - TemplateEngine.#commit(item.result); - } - } else { - const cursors = TemplateEngine.#createCursors(node); - TemplateEngine.#ready(result); - TemplateEngine.#commit(result); - TemplateEngine.#inject(result, cursors.node, { before: true }); - const item = { id, result, ...cursors }; - state.map.set(id, item); - } - const item = state.map.get(id); - const referenceNode = lastItem ? lastItem.node.nextSibling : startNode.nextSibling; - if (referenceNode !== item.startNode) { - const nodesToMove = [item.startNode]; - while (nodesToMove[nodesToMove.length - 1] !== item.node) { - nodesToMove.push(nodesToMove[nodesToMove.length - 1].nextSibling); - } - TemplateEngine.#insertAllBefore(referenceNode.parentNode, referenceNode, nodesToMove); - } - TemplateEngine.#commit(item.result); - ids.add(item.id); - lastItem = item; - } else { - throw new Error(`Unexpected ${name} value "${reference}" provided by callback.`); - } - index++; - } - for (const [id, item] of state.map.entries()) { - if (!ids.has(id)) { - TemplateEngine.#removeThrough(item.startNode, item.node); - state.map.delete(id); - } - } - } - } - static #map(node, startNode, value, identify, callback) { - TemplateEngine.#mapInner(node, startNode, identify, callback, value, 'map'); + TemplateEngine.#mapInputs(node, startNode, identify, callback, value, 'map'); } // Deprecated. Will remove in future release. static #repeat(node, startNode, value, identify, callback) { - TemplateEngine.#mapInner(node, startNode, identify, callback, value, 'repeat'); + TemplateEngine.#mapInputs(node, startNode, identify, callback, value, 'repeat'); } // Walk through each string from our tagged template function “strings” array @@ -1722,29 +1631,77 @@ 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.`); - } - result.readied = true; - const { type, strings } = result; - const analysis = TemplateEngine.#setIfMissing(TemplateEngine.#stringsToAnalysis, strings, () => ({})); - if (!analysis.done) { - analysis.done = true; - const fragment = TemplateEngine.#createFragment(type, strings); - const lookups = TemplateEngine.#findLookups(fragment); - Object.assign(analysis, { fragment, lookups }); + // Loops over given inputs to either create-or-update a list of nodes. + static #mapInputs(node, startNode, identify, callback, inputs, name) { + const state = TemplateEngine.#setIfMissing(TemplateEngine.#nodeToArrayState, startNode, () => ({})); + if (!state.map) { + // There is no mapping in our state — we have a clean slate to work with. + TemplateEngine.#clearObject(state); + state.map = new Map(); + let index = 0; + for (const input of inputs) { + const reference = callback ? callback(input, index) : input; + const result = TemplateEngine.#symbolToResult.get(reference); + if (result) { + const id = identify ? identify(input, index) : String(index); + const cursors = TemplateEngine.#createCursors(node); + TemplateEngine.#inject(result, cursors.node, true); + state.map.set(id, { id, result, ...cursors }); + } else { + throw new Error(`Unexpected ${name} value "${reference}" provided by callback.`); + } + index++; + } + } else { + // A mapping has already been created — we need to update the items. + let lastItem; + const ids = new Set(); + let index = 0; + for (const input of inputs) { + const reference = callback ? callback(input, index) : input; + const result = TemplateEngine.#symbolToResult.get(reference); + if (result) { + const id = identify ? identify(input, index) : String(index); + if (state.map.has(id)) { + const item = state.map.get(id); + if (TemplateEngine.#cannotReuseResult(item.result, result)) { + // Add new comment cursors before removing old comment cursors. + const cursors = TemplateEngine.#createCursors(item.startNode); + TemplateEngine.#removeThrough(item.startNode, item.node); + TemplateEngine.#inject(result, cursors.node, true); + Object.assign(item, { result, ...cursors }); + } else { + TemplateEngine.#update(item.result, result); + } + } else { + const cursors = TemplateEngine.#createCursors(node); + TemplateEngine.#inject(result, cursors.node, true); + const item = { id, result, ...cursors }; + state.map.set(id, item); + } + const item = state.map.get(id); + const referenceNode = lastItem ? lastItem.node.nextSibling : startNode.nextSibling; + if (referenceNode !== item.startNode) { + const nodesToMove = [item.startNode]; + while (nodesToMove[nodesToMove.length - 1] !== item.node) { + nodesToMove.push(nodesToMove[nodesToMove.length - 1].nextSibling); + } + TemplateEngine.#insertAllBefore(referenceNode.parentNode, referenceNode, nodesToMove); + } + ids.add(item.id); + lastItem = item; + } else { + throw new Error(`Unexpected ${name} value "${reference}" provided by callback.`); + } + index++; + } + for (const [id, item] of state.map.entries()) { + if (!ids.has(id)) { + TemplateEngine.#removeThrough(item.startNode, item.node); + state.map.delete(id); + } + } } - const fragment = analysis.fragment.cloneNode(true); - const targets = TemplateEngine.#findTargets(fragment, analysis.lookups); - const entries = Object.entries(targets); - Object.assign(result, { fragment, entries }); - } - - static #assign(result, newResult) { - result.lastValues = result.values; - result.values = newResult.values; } static #commitAttribute(node, name, value, lastValue) { @@ -1835,7 +1792,7 @@ class TemplateEngine { TemplateEngine.#repeat(node, startNode, update.value, update.identify, update.callback); break; case TemplateEngine.#unsafe: - TemplateEngine.#unsafe(node, startNode, update.value, lastUpdate?.value, update.language); + TemplateEngine.#unsafe(node, startNode, update.value, lastUpdate?.value); break; case TemplateEngine.#unsafeHTML: TemplateEngine.#unsafeHTML(node, startNode, update.value, lastUpdate?.value); @@ -1855,16 +1812,13 @@ class TemplateEngine { if (TemplateEngine.#cannotReuseResult(state.result, result)) { TemplateEngine.#removeBetween(startNode, node); TemplateEngine.#clearObject(state); - TemplateEngine.#ready(result); - TemplateEngine.#commit(result); - TemplateEngine.#inject(result, node, { before: true }); + TemplateEngine.#inject(result, node, true); state.result = result; } else { - TemplateEngine.#assign(state.result, result); - TemplateEngine.#commit(state.result); + TemplateEngine.#update(state.result, result); } } else if (Array.isArray(value)) { - TemplateEngine.#mapInner(node, startNode, null, null, value, 'array'); + TemplateEngine.#mapInputs(node, startNode, null, null, value, 'array'); } else { const state = TemplateEngine.#setIfMissing(TemplateEngine.#nodeToArrayState, startNode, () => ({})); if (state.result) { @@ -1918,18 +1872,51 @@ 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) { + // Inject a given result into a node for the first time. If we’ve never seen + // the template “strings” before, we also have to generate html, parse it, + // and find out binding targets. Then, we commit the values by iterating over + // our targets. Finally, we actually attach our new DOM into our node. + static #inject(result, node, before) { + // If we see the _exact_ same result again… that’s an error. We don’t allow + // integrators to reuse template results. + if (result.readied) { + throw new Error(`Unexpected re-injection of template result.`); + } + + // Create and prepare a document fragment to be injected. + result.readied = true; + const { type, strings } = result; + const analysis = TemplateEngine.#setIfMissing(TemplateEngine.#stringsToAnalysis, strings, () => ({})); + if (!analysis.done) { + analysis.done = true; + const fragment = TemplateEngine.#createFragment(type, strings); + const lookups = TemplateEngine.#findLookups(fragment); + Object.assign(analysis, { fragment, lookups }); + } + const fragment = analysis.fragment.cloneNode(true); + const targets = TemplateEngine.#findTargets(fragment, analysis.lookups); + const entries = Object.entries(targets); + Object.assign(result, { fragment, entries }); + + // Bind values via our live targets into our disconnected DOM. + TemplateEngine.#commit(result); + + // Attach a document fragment into the node. Note that all the DOM in the + // fragment will already have values correctly committed on the line above. const nodes = result.type === 'svg' ? result.fragment.firstChild.childNodes : result.fragment.childNodes; - options?.before + before ? TemplateEngine.#insertAllBefore(node.parentNode, node, nodes) : TemplateEngine.#insertAllBefore(node, null, nodes); result.fragment = null; } + static #update(result, newResult) { + Object.assign(result, { lastValues: result.values, values: newResult.values }); + TemplateEngine.#commit(result); + } + static #throwUpdaterError(updater, type) { switch (updater) { case TemplateEngine.#live: