Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: create unified API for parsing markers #5727

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "feat: create unified API for parsing markers",
"packageName": "@microsoft/fast-element",
"email": "[email protected]",
"dependentChangeType": "patch"
}
11 changes: 4 additions & 7 deletions packages/web-components/fast-element/docs/api-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -459,7 +457,7 @@ export const oneTime: BindingConfig<DefaultBindingOptions> & 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
Expand Down Expand Up @@ -502,14 +500,14 @@ export class RepeatBehavior<TSource = any> implements Behavior, Subscriber {
// @internal (undocumented)
handleChange(source: any, args: Splice[]): void;
unbind(): void;
}
}

// @public
export class RepeatDirective<TSource = any> extends HTMLDirective {
constructor(itemsBinding: Binding, templateBinding: Binding<TSource, SyntheticViewTemplate>, options: RepeatOptions);
createBehavior(targets: ViewBehaviorTargets): RepeatBehavior<TSource>;
createPlaceholder: (index: number) => string;
}
}

// @public
export interface RepeatOptions {
Expand Down Expand Up @@ -656,7 +654,6 @@ export function volatile(target: {}, name: string | Accessor, descriptor: Proper
// @public
export function when<TSource = any, TReturn = any>(binding: Binding<TSource, TReturn>, templateOrTemplateBinding: SyntheticViewTemplate | Binding<TSource, SyntheticViewTemplate>): CaptureType<TSource>;


// (No @packageDocumentation comment for this package)

```
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
32 changes: 15 additions & 17 deletions packages/web-components/fast-element/src/templating/compiler.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand All @@ -244,6 +234,14 @@ function compileNode(
return next;
}

function isMarker(node: Node, directives: ReadonlyArray<HTMLDirective>): 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.
Expand Down Expand Up @@ -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);
}
Expand Down
35 changes: 11 additions & 24 deletions packages/web-components/fast-element/src/templating/markup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -53,15 +48,7 @@ export const Markup = Object.freeze({
* Used internally by structural directives such as `repeat`.
*/
comment(index: number): string {
return `<!--${marker}:${index}-->`;
},

/**
* 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 `<!--${interpolationStart}${index}${interpolationEnd}-->`;
},
});

Expand All @@ -82,33 +69,33 @@ 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;

if (index === -1) {
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;
},

/**
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { InlinableHTMLDirective } from "@microsoft/fast-element";
import { HTMLDirective } from "@microsoft/fast-element";
import { AttributeType } from "./attributes.js";

/**
Expand Down Expand Up @@ -62,15 +62,15 @@ export type CustomElementShadowOp = {
*/
export type DirectiveOp = {
type: OpType.directive;
directive: InlinableHTMLDirective;
directive: HTMLDirective;
};

/**
* Operation to emit a bound attribute
*/
export type AttributeBindingOp = {
type: OpType.attributeBinding;
directive: InlinableHTMLDirective;
directive: HTMLDirective;
name: string;
attributeType: AttributeType;
useCustomElementInstance: boolean;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {

Expand Down