Skip to content

Commit

Permalink
Ditch unecessary “weak maps”.
Browse files Browse the repository at this point in the history
The search set for the weak maps we’re using gets untenable since it’s
a flat list of pointers for _all_ results.

It feels like a reasonable concession to hang data off of a unique
symbol key. These are non-enumerable (unless specifically trying to
enumerate symbols), which feels internal enough.

Additionally, we can just return frozen results from our tagged template
functions — it’s just passing back information from the user, so it’s
hardly even a “leak” of our inner abstractions.
  • Loading branch information
theengineear committed Nov 23, 2024
1 parent 769e656 commit c5a3540
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 138 deletions.
65 changes: 39 additions & 26 deletions test/test-template-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,27 @@ describe('html rendering', () => {
assert(container.children[0].localName === 'textarea');
container.remove();
});

it('renders the same template result multiple times for', () => {
const rawResult = html`<div id="target"></div>`;
const container1 = document.createElement('div');
const container2 = document.createElement('div');
document.body.append(container1, container2);
render(container1, rawResult);
render(container2, rawResult);
assert(!!container1.querySelector('#target'));
assert(!!container2.querySelector('#target'));
render(container1, null);
render(container2, null);
assert(!container1.querySelector('#target'));
assert(!container2.querySelector('#target'));
render(container1, rawResult);
render(container2, rawResult);
assert(!!container1.querySelector('#target'));
assert(!!container2.querySelector('#target'));
container1.remove();
container2.remove();
});
});

describe('html updaters', () => {
Expand Down Expand Up @@ -1081,6 +1102,16 @@ describe('svg updaters', () => {

describe('rendering errors', () => {
describe('templating', () => {
it('throws when given container is not a node', () => {
let error;
try {
render({}, html``);
} catch (e) {
error = e;
}
assert(error?.message === 'Unexpected non-node render container "[object Object]".', error.message);
});

it('throws when attempting to interpolate within a style tag', () => {
const getTemplate = ({ color }) => {
return html`
Expand Down Expand Up @@ -1156,12 +1187,12 @@ describe('rendering errors', () => {
});

it('throws for unquoted attributes', () => {
const templateResultReference = html`<div id="target" not-ok=${'foo'}>Gotta double-quote those.</div>`;
const rawResult = html`<div id="target" not-ok=${'foo'}>Gotta double-quote those.</div>`;
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, templateResultReference);
render(container, rawResult);
} catch (e) {
error = e;
}
Expand All @@ -1170,12 +1201,12 @@ describe('rendering errors', () => {
});

it('throws for single-quoted attributes', () => {
const templateResultReference = html`\n<div id="target" not-ok='${'foo'}'>Gotta double-quote those.</div>`;
const rawResult = html`\n<div id="target" not-ok='${'foo'}'>Gotta double-quote those.</div>`;
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, templateResultReference);
render(container, rawResult);
} catch (e) {
error = e;
}
Expand All @@ -1184,12 +1215,12 @@ describe('rendering errors', () => {
});

it('throws for unquoted properties', () => {
const templateResultReference = html`\n\n\n<div id="target" .notOk=${'foo'}>Gotta double-quote those.</div>`;
const rawResult = html`\n\n\n<div id="target" .notOk=${'foo'}>Gotta double-quote those.</div>`;
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, templateResultReference);
render(container, rawResult);
} catch (e) {
error = e;
}
Expand All @@ -1198,12 +1229,12 @@ describe('rendering errors', () => {
});

it('throws for single-quoted properties', () => {
const templateResultReference = html`<div id="target" .notOk='${'foo'}'>Gotta double-quote those.</div>`;
const rawResult = html`<div id="target" .notOk='${'foo'}'>Gotta double-quote those.</div>`;
const container = document.createElement('div');
document.body.append(container);
let error;
try {
render(container, templateResultReference);
render(container, rawResult);
} catch (e) {
error = e;
}
Expand All @@ -1228,24 +1259,6 @@ describe('rendering errors', () => {
assert(actual === expected, actual);
container.remove();
});

it('throws for re-injection of template result', () => {
const templateResultReference = html`<div id="target"></div>`;
const container = document.createElement('div');
document.body.append(container);
render(container, templateResultReference);
assert(!!container.querySelector('#target'));
render(container, null);
assert(!container.querySelector('#target'));
let error;
try {
render(container, templateResultReference);
} catch (e) {
error = e;
}
assert(error?.message === 'Unexpected re-injection of template result.', error.message);
container.remove();
});
});

describe('ifDefined', () => {
Expand Down
Loading

0 comments on commit c5a3540

Please sign in to comment.