From db133236dd8234113a38a76127130e0034dea20a Mon Sep 17 00:00:00 2001 From: Rob Eisenberg Date: Mon, 14 Mar 2022 13:29:12 -0400 Subject: [PATCH] feat: create unified API for parsing markers (#5727) * feat: create unified API for parsing markers * fix: correct logic for inserting stable first node * Change files * refactor: tersify compiler comment first node check * fix: correct some API types on the Parser * change directive op type to use return type of Parser.aggregate() Co-authored-by: EisenbergEffect Co-authored-by: nicholasrice --- ...-0e2b9162-79c1-43de-adf5-13989cd29ee1.json | 7 ++++ .../fast-element/docs/api-report.md | 11 +++--- .../src/components/controller.spec.ts | 2 +- .../fast-element/src/templating/compiler.ts | 32 ++++++++--------- .../fast-element/src/templating/markup.ts | 35 ++++++------------- .../fast-ssr/src/template-parser/op-codes.ts | 6 ++-- .../template-parser/template-parser.spec.ts | 1 - 7 files changed, 41 insertions(+), 53 deletions(-) create mode 100644 change/@microsoft-fast-element-0e2b9162-79c1-43de-adf5-13989cd29ee1.json diff --git a/change/@microsoft-fast-element-0e2b9162-79c1-43de-adf5-13989cd29ee1.json b/change/@microsoft-fast-element-0e2b9162-79c1-43de-adf5-13989cd29ee1.json new file mode 100644 index 00000000000..95d0f81a395 --- /dev/null +++ b/change/@microsoft-fast-element-0e2b9162-79c1-43de-adf5-13989cd29ee1.json @@ -0,0 +1,7 @@ +{ + "type": "major", + "comment": "feat: create unified API for parsing markers", + "packageName": "@microsoft/fast-element", + "email": "roeisenb@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/web-components/fast-element/docs/api-report.md b/packages/web-components/fast-element/docs/api-report.md index 61d4c91a073..7a6863579ec 100644 --- a/packages/web-components/fast-element/docs/api-report.md +++ b/packages/web-components/fast-element/docs/api-report.md @@ -49,7 +49,7 @@ export class AttributeDefinition implements Accessor { onAttributeChangedCallback(element: HTMLElement, value: any): void; readonly Owner: Function; setValue(source: HTMLElement, newValue: any): void; - } +} // @public export type AttributeMode = "reflect" | "boolean" | "fromView"; @@ -395,11 +395,9 @@ export const enum KernelServiceId { // @public export const Markup: Readonly<{ - marker: string; interpolation(index: number): string; attribute(index: number): string; comment(index: number): string; - indexFromComment(node: Comment): number; }>; // Warning: (ae-internal-missing-underscore) The name "Mutable" should be prefixed with an underscore because the declaration is marked as @internal @@ -459,7 +457,7 @@ export const oneTime: BindingConfig & BindingConfigResolv // @public export const Parser: Readonly<{ parse(value: string, directives: readonly HTMLDirective[]): (string | HTMLDirective)[] | null; - aggregate(parts: (string | HTMLDirective)[]): InlinableHTMLDirective; + aggregate(parts: (string | HTMLDirective)[]): HTMLDirective; }>; // @public @@ -502,14 +500,14 @@ export class RepeatBehavior implements Behavior, Subscriber { // @internal (undocumented) handleChange(source: any, args: Splice[]): void; unbind(): void; - } +} // @public export class RepeatDirective extends HTMLDirective { constructor(itemsBinding: Binding, templateBinding: Binding, options: RepeatOptions); createBehavior(targets: ViewBehaviorTargets): RepeatBehavior; createPlaceholder: (index: number) => string; - } +} // @public export interface RepeatOptions { @@ -656,7 +654,6 @@ export function volatile(target: {}, name: string | Accessor, descriptor: Proper // @public export function when(binding: Binding, templateOrTemplateBinding: SyntheticViewTemplate | Binding): CaptureType; - // (No @packageDocumentation comment for this package) ``` diff --git a/packages/web-components/fast-element/src/components/controller.spec.ts b/packages/web-components/fast-element/src/components/controller.spec.ts index 33bcd7a35da..953f2418c16 100644 --- a/packages/web-components/fast-element/src/components/controller.spec.ts +++ b/packages/web-components/fast-element/src/components/controller.spec.ts @@ -531,7 +531,7 @@ describe("The Controller", () => { } ).define(); - expect(root.innerHTML).to.equal("Test 2"); + expect(root.innerHTML).to.equal("Test 2"); document.body.removeChild(element); }); diff --git a/packages/web-components/fast-element/src/templating/compiler.ts b/packages/web-components/fast-element/src/templating/compiler.ts index abbfdf8efe9..f2e57322d95 100644 --- a/packages/web-components/fast-element/src/templating/compiler.ts +++ b/packages/web-components/fast-element/src/templating/compiler.ts @@ -1,6 +1,6 @@ import { isString } from "../interfaces.js"; import { DOM } from "../dom.js"; -import { Markup, Parser } from "./markup.js"; +import { Parser } from "./markup.js"; import { bind, oneTime } from "./binding.js"; import type { AspectedHTMLDirective, @@ -111,12 +111,6 @@ class CompilationContext implements HTMLTemplateCompilationResult { } } -const marker = Markup.marker; - -function isMarker(node: Node): node is Comment { - return node && node.nodeType === 8 && (node as Comment).data.startsWith(marker); -} - function compileAttributes( context: CompilationContext, parentId: string, @@ -228,13 +222,9 @@ function compileNode( case 3: // text node return compileContent(context, node as Text, parentId, nodeId, nodeIndex); case 8: // comment - if (isMarker(node)) { - context.addFactory( - context.directives[Markup.indexFromComment(node)], - parentId, - nodeId, - nodeIndex - ); + const parts = Parser.parse((node as Comment).data, context.directives); + if (parts !== null) { + context.addFactory(Parser.aggregate(parts), parentId, nodeId, nodeIndex); } break; } @@ -244,6 +234,14 @@ function compileNode( return next; } +function isMarker(node: Node, directives: ReadonlyArray): boolean { + return ( + node && + node.nodeType == 8 && + Parser.parse((node as Comment).data, directives) !== null + ); +} + /** * Compiles a template and associated directives into a compilation * result which can be used to create views. @@ -284,11 +282,11 @@ export function compileTemplate( // because something like a when, repeat, etc. could add nodes before the marker. // To mitigate this, we insert a stable first node. However, if we insert a node, // that will alter the result of the TreeWalker. So, we also need to offset the target index. - isMarker(fragment.firstChild!) || - // Or if there is only one node, it means the template's content + isMarker(fragment.firstChild!, directives) || + // Or if there is only one node and a directive, it means the template's content // is *only* the directive. In that case, HTMLView.dispose() misses any nodes inserted by // the directive. Inserting a new node ensures proper disposal of nodes added by the directive. - fragment.childNodes.length === 1 + (fragment.childNodes.length === 1 && directives.length) ) { fragment.insertBefore(document.createComment(""), fragment.firstChild); } diff --git a/packages/web-components/fast-element/src/templating/markup.ts b/packages/web-components/fast-element/src/templating/markup.ts index 8c66bcf54cd..327c9fe86d5 100644 --- a/packages/web-components/fast-element/src/templating/markup.ts +++ b/packages/web-components/fast-element/src/templating/markup.ts @@ -18,11 +18,6 @@ export const nextId = (): string => `${marker}-${++id}`; * @public */ export const Markup = Object.freeze({ - /** - * Gets the unique marker used by FAST to annotate templates. - */ - marker, - /** * Creates a placeholder string suitable for marking out a location *within* * an attribute value or HTML content. @@ -53,15 +48,7 @@ export const Markup = Object.freeze({ * Used internally by structural directives such as `repeat`. */ comment(index: number): string { - return ``; - }, - - /** - * Given a marker node, extract the {@link HTMLDirective} index from the placeholder. - * @param node - The marker node to extract the index from. - */ - indexFromComment(node: Comment): number { - return parseInt(node.data.replace(`${marker}:`, "")); + return ``; }, }); @@ -82,16 +69,16 @@ export const Parser = Object.freeze({ value: string, directives: readonly HTMLDirective[] ): (string | HTMLDirective)[] | null { - const valueParts = value.split(interpolationStart); + const parts = value.split(interpolationStart); - if (valueParts.length === 1) { + if (parts.length === 1) { return null; } - const bindingParts: (string | HTMLDirective)[] = []; + const result: (string | HTMLDirective)[] = []; - for (let i = 0, ii = valueParts.length; i < ii; ++i) { - const current = valueParts[i]; + for (let i = 0, ii = parts.length; i < ii; ++i) { + const current = parts[i]; const index = current.indexOf(interpolationEnd); let literal: string | HTMLDirective; @@ -99,16 +86,16 @@ export const Parser = Object.freeze({ literal = current; } else { const directiveIndex = parseInt(current.substring(0, index)); - bindingParts.push(directives[directiveIndex]); + result.push(directives[directiveIndex]); literal = current.substring(index + interpolationEndLength); } if (literal !== "") { - bindingParts.push(literal); + result.push(literal); } } - return bindingParts; + return result; }, /** @@ -117,9 +104,9 @@ export const Parser = Object.freeze({ * directives. * @returns A single inline directive that aggregates the behavior of all the parts. */ - aggregate(parts: (string | HTMLDirective)[]): InlinableHTMLDirective { + aggregate(parts: (string | HTMLDirective)[]): HTMLDirective { if (parts.length === 1) { - return parts[0] as InlinableHTMLDirective; + return parts[0] as HTMLDirective; } let aspect: string | undefined; diff --git a/packages/web-components/fast-ssr/src/template-parser/op-codes.ts b/packages/web-components/fast-ssr/src/template-parser/op-codes.ts index 669711ca992..18c1ee88d25 100644 --- a/packages/web-components/fast-ssr/src/template-parser/op-codes.ts +++ b/packages/web-components/fast-ssr/src/template-parser/op-codes.ts @@ -1,4 +1,4 @@ -import { InlinableHTMLDirective } from "@microsoft/fast-element"; +import { HTMLDirective } from "@microsoft/fast-element"; import { AttributeType } from "./attributes.js"; /** @@ -62,7 +62,7 @@ export type CustomElementShadowOp = { */ export type DirectiveOp = { type: OpType.directive; - directive: InlinableHTMLDirective; + directive: HTMLDirective; }; /** @@ -70,7 +70,7 @@ export type DirectiveOp = { */ export type AttributeBindingOp = { type: OpType.attributeBinding; - directive: InlinableHTMLDirective; + directive: HTMLDirective; name: string; attributeType: AttributeType; useCustomElementInstance: boolean; diff --git a/packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts b/packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts index dc50af0d681..c6287537b2f 100644 --- a/packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts +++ b/packages/web-components/fast-ssr/src/template-parser/template-parser.spec.ts @@ -38,7 +38,6 @@ test.describe("parseTemplateToOpCodes", () => { const code = codes[0] as DirectiveOp; expect(codes.length).toBe(1); expect(code.type).toBe(OpType.directive); - expect(code.directive.binding(null, defaultExecutionContext)).toBe("Hello World.") }); test("should sandwich directive ops between text ops when binding native element content", () => {