Skip to content

Commit

Permalink
Improve template issue forensics.
Browse files Browse the repository at this point in the history
The major changes can be boiled down into the following additions:
* Print the registered custom element tag name in the error.
* Print the approximate template line number in the error.†

† Note that this isn’t the _file line number_, just a naive count of
  the number of newlines encountered so far in the given template.

Closes #201.
  • Loading branch information
theengineear committed Nov 10, 2024
1 parent b7d1b42 commit d8f5866
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 11 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- Template errors now include approximate line numbers from the offending
template. They also print the registered custom element tag name (#201).

## [1.1.1] - 2024-11-09

### Fixed
Expand Down
4 changes: 2 additions & 2 deletions test/test-initialization-errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ it('errors are thrown in connectedCallback when template result fails to render'
try {
el.connectedCallback();
} catch (error) {
const expected = 'Invalid template for "TestElement" at path "test-element-3"';
const expected = 'Invalid template for "TestElement" / <test-element-3> at path "test-element-3".';
message = error.message;
passed = error.message.endsWith(expected);
}
Expand Down Expand Up @@ -142,7 +142,7 @@ it('errors are thrown in connectedCallback when template result fails to render
try {
el.connectedCallback();
} catch (error) {
const expected = 'Invalid template for "TestElement" at path "test-element-4[id="testing"][class="foo bar"][boolean][variation="primary"]"';
const expected = 'Invalid template for "TestElement" / <test-element-4> at path "test-element-4[id="testing"][class="foo bar"][boolean][variation="primary"]".';
message = error.message;
passed = error.message.endsWith(expected);
}
Expand Down
15 changes: 9 additions & 6 deletions test/test-template-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,7 @@ describe('rendering errors', () => {
container.remove();
});

// Note that this is line “1” because it’s perfectly inlined.
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 All @@ -856,12 +857,14 @@ describe('rendering errors', () => {
} catch (e) {
error = e;
}
assert(error?.message === `Found invalid template string "<div id="target" not-ok=" at " not-ok=".`, error.message);
assert(error?.message === `Found invalid template on or after line 1 in substring \`<div id="target" not-ok=\`. Failed to parse \` not-ok=\`.`, error.message);
container.remove();
});

// Note that this is _still_ line “1” because it begins with a newline. This
// is what authors will expect as the “first line”.
it('throws for single-quoted attributes', () => {
const templateResultReference = html`<div id="target" not-ok='${'foo'}'>Gotta double-quote those.</div>`;
const templateResultReference = html`\n<div id="target" not-ok='${'foo'}'>Gotta double-quote those.</div>`;
const container = document.createElement('div');
document.body.append(container);
let error;
Expand All @@ -870,12 +873,12 @@ describe('rendering errors', () => {
} catch (e) {
error = e;
}
assert(error?.message === `Found invalid template string "<div id="target" not-ok='" at " not-ok='".`, error.message);
assert(error?.message === `Found invalid template on or after line 1 in substring \`\n<div id="target" not-ok='\`. Failed to parse \` not-ok='\`.`, error.message);
container.remove();
});

it('throws for unquoted properties', () => {
const templateResultReference = html`<div id="target" .notOk=${'foo'}>Gotta double-quote those.</div>`;
const templateResultReference = 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;
Expand All @@ -884,7 +887,7 @@ describe('rendering errors', () => {
} catch (e) {
error = e;
}
assert(error?.message === `Found invalid template string "<div id="target" .notOk=" at " .notOk=".`, error.message);
assert(error?.message === `Found invalid template on or after line 3 in substring \`\n\n\n<div id="target" .notOk=\`. Failed to parse \` .notOk=\`.`, error.message);
container.remove();
});

Expand All @@ -898,7 +901,7 @@ describe('rendering errors', () => {
} catch (e) {
error = e;
}
assert(error?.message === `Found invalid template string "<div id="target" .notOk='" at " .notOk='".`, error.message);
assert(error?.message === `Found invalid template on or after line 1 in substring \`<div id="target" .notOk='\`. Failed to parse \` .notOk='\`.`, error.message);
container.remove();
});

Expand Down
2 changes: 1 addition & 1 deletion x-element.d.ts.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 11 additions & 2 deletions x-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,9 @@ export default class XElement extends HTMLElement {
render(renderRoot, result);
} catch (error) {
const pathString = XElement.#toPathString(this);
const message = `${error.message} — Invalid template for "${this.constructor.name}" at path "${pathString}"`;
// @ts-ignore — TypeScript doesn’t get that this can accept any class.
const tagName = customElements.getName(this.constructor);
const message = `Invalid template for "${this.constructor.name}" / <${tagName}> at path "${pathString}".`;
throw new Error(message, { cause: error });
}
}
Expand Down Expand Up @@ -1385,7 +1387,14 @@ class TemplateEngine {
string = string.slice(0, -3 - name.length) + `${TemplateEngine.#ATTRIBUTE_PREFIXES.property}${iii}="${name}`;
state.index = 1; // Accounts for an expected quote character next.
} else {
throw new Error(`Found invalid template string "${string}" at "${string.slice(state.index)}".`);
// We try and intuit what authors consider line “1”.
const handled = [...strings.slice(0, iii), string.slice(0, state.index)].join('');
const lineCount = handled.startsWith('\n')
? handled.split('\n').length - 1
: handled.split('\n').length;
// We say “on or after” because interpolated JS could add lines we
// cannot know about or account for.
throw new Error(`Found invalid template on or after line ${lineCount} in substring \`${string}\`. Failed to parse \`${string.slice(state.index)}\`.`);
}
}
} else {
Expand Down

0 comments on commit d8f5866

Please sign in to comment.