diff --git a/change/@microsoft-fast-element-77259811-4feb-4f5f-823a-8c3ee4459850.json b/change/@microsoft-fast-element-77259811-4feb-4f5f-823a-8c3ee4459850.json new file mode 100644 index 00000000000..b6814a82a83 --- /dev/null +++ b/change/@microsoft-fast-element-77259811-4feb-4f5f-823a-8c3ee4459850.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Add strict, quote-aware event argument parsing with string, number, boolean and null literal support to declarative event bindings.", + "packageName": "@microsoft/fast-element", + "email": "144495202+AKnassa@users.noreply.github.com", + "dependentChangeType": "none" +} diff --git a/packages/fast-element/SIZES.md b/packages/fast-element/SIZES.md index 92d854e5126..048a6daaa9c 100644 --- a/packages/fast-element/SIZES.md +++ b/packages/fast-element/SIZES.md @@ -19,7 +19,7 @@ Bundle sizes for `@microsoft/fast-element` exports. | repeat (@microsoft/fast-element/repeat.js) | 31.80 KB | 10.00 KB | 9.03 KB | | css (@microsoft/fast-element/css.js) | 2.43 KB | 1.00 KB | 911 B | | enableHydration (@microsoft/fast-element/hydration.js) | 46.53 KB | 13.91 KB | 12.49 KB | -| declarativeTemplate (@microsoft/fast-element/declarative.js) | 62.15 KB | 19.46 KB | 17.42 KB | +| declarativeTemplate (@microsoft/fast-element/declarative.js) | 63.94 KB | 20.10 KB | 17.98 KB | | ArrayObserver (@microsoft/fast-element/arrays.js) | 12.55 KB | 4.46 KB | 4.03 KB | | observerMap (@microsoft/fast-element/observer-map.js) | 21.96 KB | 7.73 KB | 6.97 KB | | attributeMap (@microsoft/fast-element/attribute-map.js) | 15.31 KB | 5.41 KB | 4.88 KB | diff --git a/packages/fast-element/docs/declarative/syntax.md b/packages/fast-element/docs/declarative/syntax.md index f2f7fff4b9b..b6bd2cb41df 100644 --- a/packages/fast-element/docs/declarative/syntax.md +++ b/packages/fast-element/docs/declarative/syntax.md @@ -286,7 +286,7 @@ Event bindings must include the `()` as well as being preceeded by `@` in keepin ``` -You can pass the DOM event object, the execution context, or both as arguments. Any other argument is treated as a binding expression and resolved against the current data source. +You can pass the DOM event object, the execution context, literal values, or binding paths as arguments. **`$e` — DOM event object (preferred):** ```html @@ -308,11 +308,24 @@ You can pass the DOM event object, the execution context, or both as arguments. ``` -**Arbitrary binding expressions** — any token that is not `$e` or `$c` is resolved as a binding path on the data source: +**Binding paths** — any unquoted token that is not `$e`, `$c` or a literal is resolved as a binding path on the data source. Inside an `` the path resolves against the repeat scope: ```html ``` +**Literals** — quoted strings (single or double quotes), numbers, `true`, `false` and `null` are passed through as values: +```html + + + +``` + +String literals support the `\0`, `\b`, `\f`, `\n`, `\r`, `\t`, `\v`, `\\`, `\'` and `\"` escape sequences. + +Argument lists are parsed strictly. Malformed handler syntax — a missing or unbalanced call, an invalid handler name, trailing text after the call, an unclosed string, an unknown escape sequence, a trailing comma, an empty argument slot, or an invalid path token — throws while the template is being parsed rather than being silently normalized. + Use `$e` for the DOM event object. ### Directives diff --git a/packages/fast-element/src/declarative/template-parser.ts b/packages/fast-element/src/declarative/template-parser.ts index 4df9c5e8993..50322f4a254 100644 --- a/packages/fast-element/src/declarative/template-parser.ts +++ b/packages/fast-element/src/declarative/template-parser.ts @@ -18,7 +18,7 @@ import { getExpressionChain, getNextBehavior, getRootPropertyName, - parseEventArgs, + parseEventHandler, type TemplateDirectiveBehaviorConfig, } from "./utilities.js"; @@ -318,14 +318,7 @@ export class TemplateParser { behaviorConfig.openingEndIndex, behaviorConfig.closingStartIndex, ); - const openingParenthesis = bindingHTML.indexOf("("); - const closingParenthesis = bindingHTML.indexOf(")"); - const propName = innerHTML.slice( - behaviorConfig.openingEndIndex, - behaviorConfig.closingStartIndex - - (closingParenthesis - openingParenthesis) - - 1, - ); + const { name: propName, args: parsedArgs } = parseEventHandler(bindingHTML); const type = "event"; rootPropertyName = getRootPropertyName( rootPropertyName, @@ -333,7 +326,6 @@ export class TemplateParser { context.parentContext, type, ); - const argsString = bindingHTML.slice(openingParenthesis + 1, closingParenthesis); const previousString = strings.previousString; const resolved = bindingResolver( previousString, @@ -353,8 +345,6 @@ export class TemplateParser { } : (x: any, _c: any) => x; - const parsedArgs = parseEventArgs(argsString); - const argResolvers = parsedArgs.map((parsedArg): ((x: any, c: any) => any) => { switch (parsedArg.type) { case "event": @@ -372,6 +362,10 @@ export class TemplateParser { context.parentContext, context.level, ); + default: { + const { value } = parsedArg; + return () => value; + } } }); diff --git a/packages/fast-element/src/declarative/utilities.spec.ts b/packages/fast-element/src/declarative/utilities.pw.spec.ts similarity index 87% rename from packages/fast-element/src/declarative/utilities.spec.ts rename to packages/fast-element/src/declarative/utilities.pw.spec.ts index 7b4f9d29e15..0d5fcba96e0 100644 --- a/packages/fast-element/src/declarative/utilities.spec.ts +++ b/packages/fast-element/src/declarative/utilities.pw.spec.ts @@ -13,6 +13,7 @@ import { getIndexOfNextMatchingTag, getNextBehavior, parseEventArgs, + parseEventHandler, pathResolver, type TemplateDirectiveBehaviorConfig, transformInnerHTML, @@ -975,5 +976,127 @@ test.describe("utilities", async () => { { type: "binding", rawArg: "foo" }, ]); }); + test("should parse a single quoted string literal", async () => { + expect(parseEventArgs("'static'")).toEqual([ + { type: "string", value: "static" }, + ]); + }); + test("should parse a double quoted string literal", async () => { + expect(parseEventArgs('"static"')).toEqual([ + { type: "string", value: "static" }, + ]); + }); + test("should parse an empty string literal", async () => { + expect(parseEventArgs("''")).toEqual([{ type: "string", value: "" }]); + }); + test("should treat a comma inside a string literal as content", async () => { + expect(parseEventArgs("'a,b'")).toEqual([{ type: "string", value: "a,b" }]); + }); + test("should treat a parenthesis inside a string literal as content", async () => { + expect(parseEventArgs("'a)b'")).toEqual([{ type: "string", value: "a)b" }]); + }); + test("should parse an escaped quote inside a string literal", async () => { + expect(parseEventArgs("'it\\'s'")).toEqual([ + { type: "string", value: "it's" }, + ]); + }); + test("should decode escape sequences inside a string literal", async () => { + expect(parseEventArgs(String.raw`'a\nb\tc\rd\\e\'f\"g'`)).toEqual([ + { type: "string", value: "a\nb\tc\rd\\e'f\"g" }, + ]); + }); + test("should throw for an unknown escape sequence", async () => { + expect(() => parseEventArgs(String.raw`'a\qb'`)).toThrow(); + }); + test("should parse number literals", async () => { + expect(parseEventArgs("1, -2, 1.5, 1e3")).toEqual([ + { type: "number", value: 1 }, + { type: "number", value: -2 }, + { type: "number", value: 1.5 }, + { type: "number", value: 1000 }, + ]); + }); + test("should parse boolean literals", async () => { + expect(parseEventArgs("true, false")).toEqual([ + { type: "boolean", value: true }, + { type: "boolean", value: false }, + ]); + }); + test("should parse a null literal", async () => { + expect(parseEventArgs("null")).toEqual([{ type: "null", value: null }]); + }); + test("should parse a mixed list of literals, paths and accessors", async () => { + expect(parseEventArgs("item.id, 'static', 1, true, null, $e")).toEqual([ + { type: "binding", rawArg: "item.id" }, + { type: "string", value: "static" }, + { type: "number", value: 1 }, + { type: "boolean", value: true }, + { type: "null", value: null }, + { type: "event" }, + ]); + }); + test("should tolerate surrounding whitespace", async () => { + expect(parseEventArgs(" $e , 'a b' ")).toEqual([ + { type: "event" }, + { type: "string", value: "a b" }, + ]); + }); + test("should throw for a trailing comma", async () => { + expect(() => parseEventArgs("$e,")).toThrow(); + }); + test("should throw for an empty argument slot", async () => { + expect(() => parseEventArgs("$e,,$c")).toThrow(); + }); + test("should throw for an unclosed string literal", async () => { + expect(() => parseEventArgs("'unclosed")).toThrow(); + }); + test("should throw for text after a string literal", async () => { + expect(() => parseEventArgs("'a'b")).toThrow(); + }); + test("should throw for an invalid path token", async () => { + expect(() => parseEventArgs("foo bar")).toThrow(); + }); + test("should throw for a malformed number", async () => { + expect(() => parseEventArgs("1abc")).toThrow(); + }); + }); + + test.describe("parseEventHandler", async () => { + test("should parse a handler without arguments", async () => { + expect(parseEventHandler("handleClick()")).toEqual({ + name: "handleClick", + args: [], + }); + }); + test("should parse a context path handler with arguments", async () => { + expect( + parseEventHandler("$c.parent.selectItem(item.id, 'from-list', $e)"), + ).toEqual({ + name: "$c.parent.selectItem", + args: [ + { type: "binding", rawArg: "item.id" }, + { type: "string", value: "from-list" }, + { type: "event" }, + ], + }); + }); + test("should find the closing parenthesis outside of a string literal", async () => { + expect(parseEventHandler("handleClick('a)b', $e)")).toEqual({ + name: "handleClick", + args: [{ type: "string", value: "a)b" }, { type: "event" }], + }); + }); + test("should throw when the function call is missing", async () => { + expect(() => parseEventHandler("handleClick")).toThrow(); + }); + test("should throw when the closing parenthesis is missing", async () => { + expect(() => parseEventHandler("handleClick($e")).toThrow(); + }); + test("should throw when there is text after the function call", async () => { + expect(() => parseEventHandler("handleClick() junk")).toThrow(); + }); + test("should throw for an invalid handler name", async () => { + expect(() => parseEventHandler("handle-click()")).toThrow(); + }); }); }); diff --git a/packages/fast-element/src/declarative/utilities.ts b/packages/fast-element/src/declarative/utilities.ts index 534301d9969..22b67065380 100644 --- a/packages/fast-element/src/declarative/utilities.ts +++ b/packages/fast-element/src/declarative/utilities.ts @@ -160,7 +160,14 @@ export { * The type of a parsed event handler argument. * @public */ -export type EventArgType = "event" | "context" | "binding"; +export type EventArgType = + | "event" + | "context" + | "binding" + | "string" + | "number" + | "boolean" + | "null"; /** * A parsed event handler argument descriptor. @@ -170,42 +177,251 @@ export interface ParsedEventArg { type: EventArgType; /** The raw argument string, present only when `type` is `"binding"`. */ rawArg?: string; + /** The literal value, present only for `"string"`, `"number"`, `"boolean"` and `"null"`. */ + value?: string | number | boolean | null; +} + +/** + * A parsed event handler binding. + * @public + */ +export interface ParsedEventHandler { + name: string; + args: ParsedEventArg[]; +} + +const eventArgPathPattern = /^\$?[a-zA-Z_][a-zA-Z0-9_]*(\.[$a-zA-Z_][a-zA-Z0-9_]*)*$/; +const eventArgNumberPattern = /^-?(0|[1-9][0-9]*)(\.[0-9]+)?([eE][+-]?[0-9]+)?$/; +const eventArgEscapes: Record = { + "0": "\0", + "\\": "\\", + "'": "'", + '"': '"', + b: "\b", + f: "\f", + n: "\n", + r: "\r", + t: "\t", + v: "\v", +}; + +/** + * Splits an event argument list on the commas that sit outside of string + * literals, so that a literal such as `'a,b'` remains a single argument. + * @param argsString - The raw arguments string from between the parentheses. + * @returns The untrimmed argument tokens. + */ +function splitEventArgs(argsString: string): string[] { + const args: string[] = []; + let current = ""; + let quote = ""; + + for (let i = 0; i < argsString.length; i++) { + const char = argsString[i]; + + if (quote !== "") { + if (char === "\\" && i + 1 < argsString.length) { + current += char + argsString[++i]; + continue; + } + + if (char === quote) { + quote = ""; + } + } else if (char === "'" || char === '"') { + quote = char; + } else if (char === ",") { + args.push(current); + current = ""; + continue; + } + + current += char; + } + + if (quote !== "") { + throw new Error(`Unclosed string literal in event arguments "${argsString}".`); + } + + args.push(current); + + return args; +} + +/** + * Unescapes a quoted string literal token and rejects any trailing characters. + * @param arg - The trimmed argument token, starting with `quote`. + * @param quote - The opening quote character. + * @returns The literal string value. + */ +function parseEventArgStringLiteral(arg: string, quote: string): string { + let value = ""; + + for (let i = 1; i < arg.length; i++) { + const char = arg[i]; + + if (char === "\\") { + const escaped = eventArgEscapes[arg[++i]]; + + if (escaped === undefined) { + throw new Error( + `Invalid escape sequence "\\${arg[i]}" in event argument "${arg}".`, + ); + } + + value += escaped; + continue; + } + + if (char === quote) { + if (i !== arg.length - 1) { + throw new Error( + `Unexpected text after the string literal in event argument "${arg}".`, + ); + } + + return value; + } + + value += char; + } + + throw new Error(`Unclosed string literal in event argument "${arg}".`); +} + +/** + * Resolves a single event argument token to its descriptor. + * @param arg - The trimmed argument token. + * @returns A {@link ParsedEventArg} descriptor. + */ +function parseEventArg(arg: string): ParsedEventArg { + switch (arg) { + case eventArgAccessor: + return { type: "event" }; + case executionContextAccessor: + return { type: "context" }; + case "true": + return { type: "boolean", value: true }; + case "false": + return { type: "boolean", value: false }; + case "null": + return { type: "null", value: null }; + } + + const quote = arg[0]; + + if (quote === "'" || quote === '"') { + return { type: "string", value: parseEventArgStringLiteral(arg, quote) }; + } + + if (eventArgNumberPattern.test(arg)) { + return { type: "number", value: Number(arg) }; + } + + if (eventArgPathPattern.test(arg)) { + return { type: "binding", rawArg: arg }; + } + + throw new Error(`Invalid event argument "${arg}".`); } /** * Parses the arguments string of an event handler binding into an array of - * typed argument descriptors. Unrecognised tokens are returned as `"binding"` - * type with their raw string preserved. + * typed argument descriptors. Malformed argument lists are rejected rather than + * normalised. * * Special arguments: * - `$e` — resolves to the DOM event object * - `$c` — resolves to the full execution context object * - * Any other token is treated as a binding path and resolved against the current - * data source. + * Quoted tokens are string literals, and `true`, `false`, `null` and numeric + * tokens are their respective literals. Any other token is treated as a binding + * path and resolved against the current data source. * * @param argsString - The raw arguments string from between the parentheses, - * e.g. `""`, `"$e"`, `"$c"`, or `"$e, $c"`. + * e.g. `""`, `"$e"`, `"$c"`, or `"item.id, 'static', 1, true, null, $e"`. * @returns An array of {@link ParsedEventArg} descriptors. * @public */ export function parseEventArgs(argsString: string): ParsedEventArg[] { if (argsString.trim() === "") return []; - return argsString - .split(",") - .map(arg => arg.trim()) - .filter(arg => arg !== "") - .map((arg): ParsedEventArg => { - switch (arg) { - case eventArgAccessor: - return { type: "event" }; - case executionContextAccessor: - return { type: "context" }; - default: - return { type: "binding", rawArg: arg }; + return splitEventArgs(argsString).map(arg => { + const trimmed = arg.trim(); + + if (trimmed === "") { + throw new Error(`Empty event argument in "${argsString}".`); + } + + return parseEventArg(trimmed); + }); +} + +/** + * Finds the index of the parenthesis closing the call opened at + * `openingParenthesis`, ignoring parentheses inside string literals. + * @param binding - The trimmed event binding expression. + * @param openingParenthesis - The index of the opening parenthesis. + * @returns The index of the closing parenthesis. + */ +function findEventArgsClose(binding: string, openingParenthesis: number): number { + let quote = ""; + + for (let i = openingParenthesis + 1; i < binding.length; i++) { + const char = binding[i]; + + if (quote !== "") { + if (char === "\\") { + i++; + } else if (char === quote) { + quote = ""; } - }); + } else if (char === "'" || char === '"') { + quote = char; + } else if (char === ")") { + return i; + } + } + + throw new Error(`Event binding "${binding}" is missing a closing parenthesis.`); +} + +/** + * Parses an event handler binding expression, e.g. + * `$c.parent.selectItem(item.id, 'from-list', $e)`, into the handler path and + * its typed argument descriptors. Handler calls that are malformed — a missing + * or unbalanced call, an invalid handler name, or trailing text — are rejected. + * + * @param bindingHTML - The event binding expression from between the braces. + * @returns A {@link ParsedEventHandler}. + * @public + */ +export function parseEventHandler(bindingHTML: string): ParsedEventHandler { + const binding = bindingHTML.trim(); + const openingParenthesis = binding.indexOf("("); + + if (openingParenthesis === -1) { + throw new Error(`Event binding "${bindingHTML}" is missing a function call.`); + } + + const closingParenthesis = findEventArgsClose(binding, openingParenthesis); + + if (binding.slice(closingParenthesis + 1).trim() !== "") { + throw new Error( + `Unexpected text after the event handler call in "${bindingHTML}".`, + ); + } + + const name = binding.slice(0, openingParenthesis).trim(); + + if (!eventArgPathPattern.test(name)) { + throw new Error(`Invalid event handler name "${name}".`); + } + + return { + name, + args: parseEventArgs(binding.slice(openingParenthesis + 1, closingParenthesis)), + }; } const startInnerHTMLDiv = `
{ expect(message).toEqual("click"); }); + test("create an event attribute with literal arguments", async ({ page }) => { + const hydrationCompleted = page.waitForFunction( + () => (window as any).hydrationCompleted === true, + ); + await page.goto("/fixtures/bindings/event/"); + await hydrationCompleted; + + const customElement = page.locator("test-element"); + + let message; + page.on("console", msg => (message = msg.text())); + + await customElement.locator("button").nth(6).click(); + + expect(message).toEqual("a,b)c,string,1.5,number,true,boolean,null,click"); + }); + test("create an event attribute with repeat scope and literal arguments", async ({ + page, + }) => { + const hydrationCompleted = page.waitForFunction( + () => (window as any).hydrationCompleted === true, + ); + await page.goto("/fixtures/bindings/event/"); + await hydrationCompleted; + + const customElement = page.locator("test-element"); + + let message; + page.on("console", msg => (message = msg.text())); + + await customElement.locator("button").nth(7).click(); + + expect(message).toEqual("id-1,from-list,click"); + + await customElement.locator("button").nth(8).click(); + + expect(message).toEqual("id-2,from-list,click"); + }); }); diff --git a/packages/fast-element/test/declarative/fixtures/bindings/event/index.html b/packages/fast-element/test/declarative/fixtures/bindings/event/index.html index f4c4e123011..fe393249e00 100644 --- a/packages/fast-element/test/declarative/fixtures/bindings/event/index.html +++ b/packages/fast-element/test/declarative/fixtures/bindings/event/index.html @@ -18,7 +18,19 @@ + + + + + + + diff --git a/packages/fast-element/test/declarative/fixtures/bindings/event/main.ts b/packages/fast-element/test/declarative/fixtures/bindings/event/main.ts index 118c8c730d8..fea49c272ed 100644 --- a/packages/fast-element/test/declarative/fixtures/bindings/event/main.ts +++ b/packages/fast-element/test/declarative/fixtures/bindings/event/main.ts @@ -2,11 +2,18 @@ import { attr } from "@microsoft/fast-element/attr.js"; import { declarativeTemplate } from "@microsoft/fast-element/declarative.js"; import { FASTElement } from "@microsoft/fast-element/fast-element.js"; import { enableHydration } from "@microsoft/fast-element/hydration.js"; +import { observable } from "@microsoft/fast-element/observable.js"; class TestElement extends FASTElement { @attr foo: string = ""; + @observable + items: Array<{ id: string; name: string }> = [ + { id: "id-1", name: "Item 1" }, + { id: "id-2", name: "Item 2" }, + ]; + public handleNoArgsClick = (): void => { console.log("no args"); }; @@ -30,6 +37,22 @@ class TestElement extends FASTElement { public handleContextEventArgClick = (e: MouseEvent): void => { console.log(e.type); }; + + public handleLiteralArgsClick = ( + text: string, + count: number, + enabled: boolean, + empty: null, + e: MouseEvent, + ): void => { + console.log( + `${text},${typeof text},${count},${typeof count},${enabled},${typeof enabled},${empty},${e.type}`, + ); + }; + + public handleRepeatArgsClick = (id: string, source: string, e: MouseEvent): void => { + console.log(`${id},${source},${e.type}`); + }; } TestElement.define({ name: "test-element", diff --git a/packages/fast-element/test/declarative/fixtures/bindings/event/state.json b/packages/fast-element/test/declarative/fixtures/bindings/event/state.json index e63d37b65a8..c84a5b322ef 100644 --- a/packages/fast-element/test/declarative/fixtures/bindings/event/state.json +++ b/packages/fast-element/test/declarative/fixtures/bindings/event/state.json @@ -1,3 +1,4 @@ { - "foo": "bar" + "foo": "bar", + "items": [{ "id": "id-1", "name": "Item 1" }, { "id": "id-2", "name": "Item 2" }] } diff --git a/packages/fast-element/test/declarative/fixtures/bindings/event/templates.html b/packages/fast-element/test/declarative/fixtures/bindings/event/templates.html index 2dc3b527f18..59b8780f7e3 100644 --- a/packages/fast-element/test/declarative/fixtures/bindings/event/templates.html +++ b/packages/fast-element/test/declarative/fixtures/bindings/event/templates.html @@ -14,5 +14,13 @@ + + + + diff --git a/sites/website/src/docs/3.x/resources/export-sizes.md b/sites/website/src/docs/3.x/resources/export-sizes.md index 8cc65070e6d..f0b70ae20d7 100644 --- a/sites/website/src/docs/3.x/resources/export-sizes.md +++ b/sites/website/src/docs/3.x/resources/export-sizes.md @@ -34,7 +34,7 @@ Bundle sizes for `@microsoft/fast-element` exports. | repeat (@microsoft/fast-element/repeat.js) | 31.80 KB | 10.00 KB | 9.03 KB | | css (@microsoft/fast-element/css.js) | 2.43 KB | 1.00 KB | 911 B | | enableHydration (@microsoft/fast-element/hydration.js) | 46.53 KB | 13.91 KB | 12.49 KB | -| declarativeTemplate (@microsoft/fast-element/declarative.js) | 62.15 KB | 19.46 KB | 17.42 KB | +| declarativeTemplate (@microsoft/fast-element/declarative.js) | 63.94 KB | 20.10 KB | 17.98 KB | | ArrayObserver (@microsoft/fast-element/arrays.js) | 12.55 KB | 4.46 KB | 4.03 KB | | observerMap (@microsoft/fast-element/observer-map.js) | 21.96 KB | 7.73 KB | 6.97 KB | | attributeMap (@microsoft/fast-element/attribute-map.js) | 15.31 KB | 5.41 KB | 4.88 KB |