Skip to content
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": "patch",
"comment": "Avoid rebinding internal attribute backing fields during connect.",
"packageName": "@microsoft/fast-element",
"email": "7559015+janechu@users.noreply.github.com",
"dependentChangeType": "none"
}
4 changes: 3 additions & 1 deletion packages/fast-element/DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ The previous `FAST.getById()` slot registry, `FASTGlobal` type, and `KernelServi
- Holds the element's `FASTElementDefinition` (name, template, styles, observed attributes).
- Manages a `Stages` state machine: `disconnected β†’ connecting β†’ connected β†’ disconnecting β†’ disconnected`.
- Exposes `isPrerendered: Promise<boolean>` which resolves to `true` when the element had a declarative shadow root (DSD) at connect time, regardless of whether hydration ran. Exposes `isHydrated: Promise<boolean>` which resolves to `true` only when hydration actually ran successfully. The `ViewController` interface also exposes both `isPrerendered` and `isHydrated` as `Promise<boolean>` for custom directives. Attribute-skip logic during the hydration bind uses an internal `_skipAttrUpdates` flag that is never exposed as a public boolean.
- On `connect()`: restores pre-upgrade observable values, synchronizes any late-defined attribute-map attributes and observes only those late attributes for future DOM changes, calls `connectedCallback` on all `HostBehavior`s, renders the current template into the shadow root when one is available, and applies styles.
- On `connect()`: restores public pre-upgrade observable values that were set as own properties, synchronizes any late-defined attribute-map attributes and observes only those late attributes for future DOM changes, calls `connectedCallback` on all `HostBehavior`s, renders the current template into the shadow root when one is available, and applies styles. Internal observable backing slots such as `_foo` are not normalized into public `foo` assignments; they already represent FAST-owned model state. Opt-in extensions such as `observerMap()` perform any additional proxying through the public accessor paths they install.
- Rendering is split into two modular paths. Hydration is pluggable: `enableHydration()` from `@microsoft/fast-element/hydration.js` installs a hook via `ElementController.installHydrationHook()`, keeping zero hydration imports in the core controller:
- **Prerendered**: The hydration hook (installed by `enableHydration()`) registers the element in the active `HydrationTracker`, swaps `onAttributeChangedCallback` to a no-op so the upgrade-time burst of callbacks is discarded, hydrates the existing DOM via `template.hydrate()`, then restores the standard handler and removes the element from the tracker. The entire method is wrapped in `try/finally` to guarantee cleanup even if an error occurs during hydration. After this point, all future attribute changes flow through the real handler with zero overhead.
- **Client-side**: `renderClientSide()` clones the compiled fragment, binds, and appends to the host β€” the standard path with no prerender logic.
Expand Down Expand Up @@ -211,6 +211,8 @@ MyComponent.define({ name: "my-component" }, [myPlugin()]);
1. Stores the new value in a backing slot.
2. Calls `Observable.getNotifier(this).notify(name)` to fan out to subscribers.

Backing slots are internal implementation details. Lifecycle rebinding treats public own properties as pre-upgrade assignments that need to flow through the accessor, but it does not reinterpret FAST-owned backing slots as public property writes.

`Observable.getNotifier(object)` returns the `PropertyChangeNotifier` for an object, creating one on demand. Notifiers are stored in a `WeakMap` so they don't prevent GC.

#### SubscriberSet / PropertyChangeNotifier
Expand Down
6 changes: 3 additions & 3 deletions packages/fast-element/SIZES.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ Bundle sizes for `@microsoft/fast-element` exports.

| Export | Minified | Gzip | Brotli |
|--------|----------|------|--------|
| CDN Rollup Bundle | 78.77 KB | 23.48 KB | 20.87 KB |
| FASTElement (@microsoft/fast-element/fast-element.js) | 23.24 KB | 7.19 KB | 6.49 KB |
| CDN Rollup Bundle | 78.61 KB | 23.40 KB | 20.77 KB |
| FASTElement (@microsoft/fast-element/fast-element.js) | 23.11 KB | 7.12 KB | 6.41 KB |
| Updates (@microsoft/fast-element/updates.js) | 473 B | 335 B | 290 B |
| Observable (@microsoft/fast-element/observable.js) | 6.75 KB | 2.51 KB | 2.23 KB |
| observable (@microsoft/fast-element/observable.js) | 6.79 KB | 2.52 KB | 2.24 KB |
Expand All @@ -18,7 +18,7 @@ Bundle sizes for `@microsoft/fast-element` exports.
| html (@microsoft/fast-element/html.js) | 27.73 KB | 8.96 KB | 8.02 KB |
| 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.66 KB | 13.99 KB | 12.56 KB |
| 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 |
| ArrayObserver (@microsoft/fast-element/arrays.js) | 12.55 KB | 4.46 KB | 4.03 KB |
| observerMap (@microsoft/fast-element/observer-map.js) | 20.08 KB | 7.15 KB | 6.43 KB |
Expand Down
11 changes: 9 additions & 2 deletions packages/fast-element/docs/architecture/fastelement.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ flowchart TD
B --> D
B --> E
B --> F
C[bind observables by capturing the Custom Elements properties and setting the values from the bound observables on the Custom Element]
C[bind observables by capturing public own Custom Element properties and setting those values through the matching accessors]
D[connect behaviors by call the <code>connectedCallback</code> on all behaviors]
E[render template, execute an <code>ElementViewTemplate</code>'s render method]
F[add styles either as an appended <code>StyleElement</code> node or an <code>adoptedStylesheet</code> which may include and attach behaviors]
Expand Down Expand Up @@ -65,4 +65,11 @@ flowchart TD
D[An <code>Updates.enqueue</code> is called which places the update in the task queue which is then executed, these are performed async unless otherwise specified]
```

These changes are observed and similar to the way `Observables` work, they utilize an `Accessor` pattern which has a `getValue` and `setValue` in which DOM updates are applied.
These changes are observed and similar to the way `Observables` work, they utilize an `Accessor` pattern which has a `getValue` and `setValue` in which DOM updates are applied.

Observable backing slots such as `_foo` are FAST-owned internal state. The
controller's connection-time observable binding only replays public own
properties that shadow prototype accessors; it does not normalize backing slots
into public property assignments. This keeps attribute converter output in its
already-converted model form while preserving pre-upgrade public property
assignments and opt-in extension behavior.
207 changes: 207 additions & 0 deletions packages/fast-element/src/components/element-controller.pw.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,213 @@ test.describe("The ElementController", () => {
});

test.describe("during connect", () => {
test("does not rebind converted attribute backing fields through converters", async ({
page,
}) => {
await page.goto("/");

const result = await page.evaluate(async () => {
// @ts-expect-error: Client module.
const { FASTElement, FASTElementDefinition, uniqueElementName } =
await import("/main.js");

const calls: Array<{ type: string; value: string }> = [];
const json = '{"title":"hello"}';
const converter = {
toView(value: any) {
return JSON.stringify(value);
},
fromView(value: any) {
calls.push({ type: typeof value, value: String(value) });
return JSON.parse(value);
},
};

const name = uniqueElementName();
(
await FASTElementDefinition.compose(
class ControllerTest extends FASTElement {
static definition = {
name,
attributes: [{ property: "data", converter }],
};
},
)
).define();

const element = document.createElement(name) as any;
element.setAttribute("data", json);
document.body.appendChild(element);

const value = {
calls,
data: element.data,
};

document.body.removeChild(element);

return value;
});

expect(result.calls).toEqual([
{ type: "string", value: '{"title":"hello"}' },
]);
expect(result.data).toEqual({ title: "hello" });
});

test("rebinds public own observable properties before rendering", async ({
page,
}) => {
await page.goto("/");

const result = await page.evaluate(async () => {
// @ts-expect-error: Client module.
const {
FASTElement,
FASTElementDefinition,
ElementController,
Observable,
html,
uniqueElementName,
} = await import("/main.js");

const changes: string[] = [];
const name = uniqueElementName();

class ControllerTest extends FASTElement {
public messageChanged(
_oldValue: string | undefined,
newValue: string,
) {
changes.push(newValue);
}
}

Observable.defineProperty(ControllerTest.prototype, "message");

(
await FASTElementDefinition.compose(ControllerTest, {
name,
template: html<any>`<span>${x => x.message}</span>`,
})
).define();

const element = document.createElement(name) as any;
const controller = ElementController.forCustomElement(element);

Object.defineProperty(element, "message", {
configurable: true,
enumerable: true,
value: "hello",
writable: true,
});

controller.connect();

const value = {
changes,
hasOwnMessage: Object.prototype.hasOwnProperty.call(
element,
"message",
),
message: element.message,
text: element.shadowRoot?.textContent ?? "",
};

controller.disconnect();

return value;
});

expect(result.changes).toEqual(["hello"]);
expect(result.hasOwnMessage).toBe(false);
expect(result.message).toBe("hello");
expect(result.text).toBe("hello");
});

test("preserves observable backing fields set on the instance", async ({
page,
}) => {
await page.goto("/");

const result = await page.evaluate(async () => {
// @ts-expect-error: Client module.
const {
FASTElement,
FASTElementDefinition,
ElementController,
Observable,
html,
uniqueElementName,
} = await import("/main.js");

const changes: Array<{ oldValue: string; newValue: string }> = [];
const name = uniqueElementName();

class ControllerTest extends FASTElement {
public _message = "hello";

public messageChanged(oldValue: string, newValue: string) {
changes.push({ oldValue, newValue });
}
}

Observable.defineProperty(ControllerTest.prototype, "message");

(
await FASTElementDefinition.compose(ControllerTest, {
name,
template: html<any>`<span>${x => x.message}</span>`,
})
).define();

const element = document.createElement(name) as any;
const controller = ElementController.forCustomElement(element);

controller.connect();

const initial = {
hasOwnBackingField: Object.prototype.hasOwnProperty.call(
element,
"_message",
),
hasOwnMessage: Object.prototype.hasOwnProperty.call(
element,
"message",
),
message: element.message,
text: element.shadowRoot?.textContent ?? "",
};

element.message = "updated";
await new Promise(resolve => requestAnimationFrame(resolve));

const updated = {
backingField: element._message,
changes,
message: element.message,
text: element.shadowRoot?.textContent ?? "",
};

controller.disconnect();

return { initial, updated };
});

expect(result.initial).toEqual({
hasOwnBackingField: true,
hasOwnMessage: false,
message: "hello",
text: "hello",
});
expect(result.updated).toEqual({
backingField: "updated",
changes: [{ oldValue: "hello", newValue: "updated" }],
message: "updated",
text: "updated",
});
});

test("renders nothing to shadow dom in shadow dom mode when there's no template", async ({
page,
}) => {
Expand Down
24 changes: 5 additions & 19 deletions packages/fast-element/src/components/element-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,8 @@ export class ElementController<TElement extends HTMLElement = HTMLElement>
}

/**
* Captures own-properties that shadow observable accessors on the prototype so
* they can be rebound through the accessor before rendering.
* Captures public own-properties that shadow observable accessors on the
* prototype so they can be rebound through the accessor before rendering.
*/
protected captureBoundObservables() {
const element = this.source;
Expand Down Expand Up @@ -564,35 +564,21 @@ export class ElementController<TElement extends HTMLElement = HTMLElement>

for (let i = 0, ii = propertyNames.length; i < ii; ++i) {
const ownPropertyName = propertyNames[i];
const propertyName = (
ownPropertyName[0] === "_" ? ownPropertyName.slice(1) : ownPropertyName
) as keyof TElement;
const propertyName = ownPropertyName as keyof TElement;

if (!hasPrototypeAccessor(propertyName as string)) {
continue;
}

const value = (element as any)[propertyName];
const isBackingField = ownPropertyName !== propertyName;
const isRebindableObject =
value !== null &&
typeof value === "object" &&
!value?.$isProxy &&
!(Array.isArray(value) && (value as any)?.$fastController);

if (value === void 0) {
if (!isBackingField) {
delete element[ownPropertyName as keyof TElement];
}

continue;
}
delete element[propertyName];

if (isBackingField && !isRebindableObject) {
continue;
}

delete element[ownPropertyName as keyof TElement];
delete element[propertyName];
(boundObservables ??= this.boundObservables = Object.create(null))[
propertyName
] = value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,65 @@ test.describe("extension subpaths", () => {
expect(result.replacedOriginal).toBe(true);
});

test("observerMap proxies public own properties rebound on connect", async ({
page,
}) => {
await page.goto("/");

const result = await page.evaluate(async () => {
// @ts-expect-error: Client module.
const { Schema, observerMap } = await import("/extension-subpaths-main.js");
// @ts-expect-error: Client module.
const { FASTElement, uniqueElementName } = await import("/main.js");

const name = uniqueElementName();
const schema = new Schema(name);
schema.addPath({
rootPropertyName: "model",
pathConfig: {
type: "default",
path: "model.value",
currentContext: null,
parentContext: null,
},
childrenMap: null,
});

class ObserverMapElement extends FASTElement {
public model: any;
}

await ObserverMapElement.define({ name, schema }, [observerMap({ schema })]);

const element = document.createElement(name) as any;
const originalValue = { value: "before" };
Object.defineProperty(element, "model", {
configurable: true,
enumerable: true,
value: originalValue,
writable: true,
});

document.body.appendChild(element);

const value = {
hasOwnModel: Object.prototype.hasOwnProperty.call(element, "model"),
isProxy: element.model.$isProxy === true,
preservesValue: element.model.value === "before",
replacedOriginal: element.model !== originalValue,
};

document.body.removeChild(element);

return value;
});

expect(result.hasOwnModel).toBe(false);
expect(result.isProxy).toBe(true);
expect(result.preservesValue).toBe(true);
expect(result.replacedOriginal).toBe(true);
});

test("observerMap observes items spliced into nested $defs arrays", async ({
page,
}) => {
Expand Down
Loading
Loading