refactor: move observerMap and attributeMap to fast-element as FASTElementExtension factories#7471
Conversation
…tExtension factories - Create packages/fast-element/src/extensions/observer-map.ts and attribute-map.ts as FASTElementExtension factory functions - Add extensions parameter to FASTElementDefinition.compose() for synchronous execution after definition construction (critical for timing with TemplateElement) - Update FASTElement.define() to accept and forward extensions array - Remove currying functions, types, and pending maps from fast-html/template.ts - Update fast-html to import types and pending registries from fast-element - Rename 'attribute-name-strategy' property to 'attributeNameStrategy' - Remove config: nesting from extension configs (pass directly) - Update all fixture files and benchmarks to new extension pattern - Update DESIGN.md, README.md, SCHEMA_OBSERVER_MAP.md, WRITING_FIXTURES.md - Regenerate API reports and website API docs - Add beachball change files for both packages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
235bcce to
6a7c20b
Compare
9b7160b to
71bcf16
Compare
71bcf16 to
3f4cca9
Compare
There was a problem hiding this comment.
Pull request overview
This PR moves the observer-map and attribute-map functionality (plus related schema/proxy utilities) from @microsoft/fast-html into @microsoft/fast-element as FASTElementExtension factories, and updates fast-html + docs/fixtures to use the new extension-based registration flow.
Changes:
- Add
extensions?: FASTElementExtension[]toFASTElementDefinition.compose()and run extensions synchronously after definition construction. - Introduce
observerMap()/attributeMap()extension factories (and related Schema/utilities) in@microsoft/fast-element; update fast-html to consume pending registries instead ofTemplateElement.options(). - Update fixtures, benchmarks, and website/API docs to reflect the new extension APIs and the
attributeNameStrategyrename.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| sites/website/src/docs/3.x/api/fast-element.observermappathnode.md | Adds generated API doc page for ObserverMapPathNode. |
| sites/website/src/docs/3.x/api/fast-element.observermappathnode._observe.md | Adds generated API doc page for ObserverMapPathNode.$observe. |
| sites/website/src/docs/3.x/api/fast-element.observermappathentry.md | Adds generated API doc page for ObserverMapPathEntry. |
| sites/website/src/docs/3.x/api/fast-element.observermapconfig.properties.md | Adds generated API doc page for ObserverMapConfig.properties. |
| sites/website/src/docs/3.x/api/fast-element.observermapconfig.md | Adds generated API doc page for ObserverMapConfig. |
| sites/website/src/docs/3.x/api/fast-element.observermap.md | Adds generated API doc page for observerMap(). |
| sites/website/src/docs/3.x/api/fast-element.attributemapconfig.md | Adds generated API doc page for AttributeMapConfig. |
| sites/website/src/docs/3.x/api/fast-element.attributemapconfig.attributenamestrategy.md | Adds generated API doc page for AttributeMapConfig.attributeNameStrategy. |
| sites/website/src/docs/3.x/api/fast-element.attributemap.md | Adds generated API doc page for attributeMap(). |
| sites/website/src/docs/3.x/api/fast-element.md | Updates fast-element API index listing to include new extension exports/types. |
| sites/website/src/docs/3.x/api/fast-element.fastelementdefinition.md | Updates API docs to reflect compose(..., extensions) signature. |
| sites/website/src/docs/3.x/api/fast-element.fastelementdefinition.compose.md | Documents new extensions parameter on FASTElementDefinition.compose(). |
| sites/benchmarks/src/scenarios/dot-syntax/csr/main.ts | Migrates benchmark scenario from TemplateElement.options() to observerMap() extension. |
| packages/fast-html/test/fixtures/scenarios/nested-elements/main.ts | Updates fixture to register observer-map via extensions and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/extensions/observer-map/main.ts | Updates fixture to use observerMap() extension and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/extensions/observer-map-properties/main.ts | Updates fixture to use selective observerMap({ properties }) extension config. |
| packages/fast-html/test/fixtures/extensions/observer-map-deep-merge/main.ts | Updates fixture imports/usage to use fast-element deepMerge + observerMap() extension. |
| packages/fast-html/test/fixtures/extensions/observer-map-config-object/main.ts | Updates fixture to use observerMap() / attributeMap() extensions and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/extensions/attribute-map/main.ts | Updates fixture to use attributeMap() extension and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/extensions/attribute-map-naming-strategy/main.ts | Updates fixture to use attributeMap({ attributeNameStrategy: \"camelCase\" }). |
| packages/fast-html/test/fixtures/extensions/README.md | Updates fixture documentation to describe the new extension-based fixtures (needs correction per comments). |
| packages/fast-html/test/fixtures/ecosystem/lifecycle-callbacks/main.ts | Updates fixture to register observer-map via extensions and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/directives/repeat/main.ts | Updates fixture to register observer-map via extensions and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/bindings/dot-syntax/main.ts | Updates fixture to register observer-map via extensions and removes TemplateElement options usage. |
| packages/fast-html/test/fixtures/WRITING_FIXTURES.md | Updates fixture authoring guide to use observerMap() extensions instead of TemplateElement options. |
| packages/fast-html/src/index.ts | Removes re-exports of observer/attribute-map types/classes from fast-html entrypoint. |
| packages/fast-html/src/components/utilities.ts | Removes observer-map/schema proxy/deep-merge utilities from fast-html (moved to fast-element). |
| packages/fast-html/src/components/utilities.spec.ts | Switches tests to import findDef from fast-element extensions utilities. |
| packages/fast-html/src/components/template.ts | Removes TemplateElement options system; consumes pendingObserverMaps/pendingAttributeMaps registries instead. |
| packages/fast-html/src/components/schema.ts | Re-exports schema types/Schema implementation from fast-element extensions. |
| packages/fast-html/src/components/observer-map.spec.ts | Switches tests to import ObserverMap from fast-element extensions. |
| packages/fast-html/src/components/index.ts | Removes component exports that were tied to the old options-based map APIs. |
| packages/fast-html/docs/api-report.api.md | Updates fast-html API report to reflect removed exports and TemplateElement API changes. |
| packages/fast-html/SCHEMA_OBSERVER_MAP.md | Updates documentation to configure ObserverMap via observerMap() extension. |
| packages/fast-html/README.md | Updates README to document observerMap()/attributeMap() extensions (needs correction per comments). |
| packages/fast-html/DESIGN.md | Updates design doc to describe observer/attribute maps as extensions rather than TemplateElement options. |
| packages/fast-element/src/index.ts | Re-exports new extension factories/classes/types/utilities from fast-element root. |
| packages/fast-element/src/extensions/utilities.ts | Adds moved proxy/deep-merge/def-lookup utilities under fast-element extensions. |
| packages/fast-element/src/extensions/schema.ts | Adds moved Schema/types under fast-element extensions. |
| packages/fast-element/src/extensions/observer-map.ts | Adds observerMap() extension factory + pending registry and marks ObserverMap as public. |
| packages/fast-element/src/extensions/attribute-map.ts | Adds attributeMap() extension factory + pending registry; renames option to attributeNameStrategy. |
| packages/fast-element/src/components/fast-element.ts | Wires extensions into compose flow and adjusts deferred-hydration define timing. |
| packages/fast-element/src/components/fast-definitions.ts | Adds extensions?: FASTElementExtension[] param to compose() and executes extensions synchronously. |
| packages/fast-element/package.json | Adds explicit subpath exports for extensions modules. |
| packages/fast-element/docs/api-report.api.md | Updates fast-element API report to include new public exports (shows new extractor warnings). |
| change/@microsoft-fast-html-94dc05b9-99f5-4387-bd18-25a250604554.json | Beachball change file for fast-html prerelease refactor. |
| change/@microsoft-fast-element-b9d26cf5-d4e3-43ad-ae88-056745023268.json | Beachball change file for fast-element minor release refactor. |
3f4cca9 to
d457492
Compare
d457492 to
1d3864b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 96 out of 96 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
packages/fast-element/src/extensions/attribute-map.ts:62
- New public extension factory
attributeMap()(and the pending registry behavior it introduces) doesn't appear to have dedicated fast-element tests. Since this is a public API surface and affects define-time behavior, adding unit/Playwright coverage in fast-element would help prevent regressions (e.g., ensures the pending config is registered and consumed exactly once, and verifies the naming strategy behavior).
packages/fast-element/src/extensions/observer-map.ts:85 - New public extension factory
observerMap()(and the pending registry behavior it introduces) doesn't appear to have dedicated fast-element tests. Since this controls schema stamping and proxy/observable wiring, consider adding fast-element unit/Playwright tests that validate config registration + consumption and a couple of representativepropertiestree cases (including$observe: falsewith re-included children).
1d3864b to
d87bfa6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 103 out of 103 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
packages/fast-element/src/extensions/observer-map.ts:86
observerMap()is now a public extension factory and is relied on by fast-html for core behavior, but there are no fast-element tests directly covering the factory/registry behavior (e.g., that the extension stores the config underdefinition.nameand thatobserverMap()with no args is treated as enabled). Adding a small unit/Playwright test in fast-element would help prevent regressions in this new public API.
packages/fast-element/src/extensions/attribute-map.ts:63attributeMap()is now a public extension factory and is relied on by fast-html for behavior, but there are no fast-element tests directly covering the factory/registry behavior and the config mapping (e.g., default strategy vsattributeNameStrategy: "camelCase"). Adding a focused test in fast-element would help prevent regressions in this new public API.
…tExtension factories - Create packages/fast-element/src/extensions/observer-map.ts and attribute-map.ts as FASTElementExtension factory functions with ObserverMap/AttributeMap classes - Create packages/fast-element/src/extensions/schema.ts with Schema class and types - Create packages/fast-element/src/extensions/utilities.ts with observable proxy utilities (assignObservables, deepMerge, assignProxy, deepEqual, etc.) - Add sub-path exports for extensions in fast-element package.json - Add extensions parameter to FASTElementDefinition.compose() for synchronous execution after definition construction (critical for timing with TemplateElement) - Update FASTElement.define() to accept and forward extensions array - Remove ObserverMap, AttributeMap classes and proxy utilities from fast-html - Replace fast-html schema.ts with re-exports from fast-element - Update all fixture files to import deepMerge from fast-element - Update DESIGN.md, README.md, SCHEMA_OBSERVER_MAP.md, WRITING_FIXTURES.md - Regenerate API reports and website API docs - Add beachball change files for both packages Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
d87bfa6 to
9660d9c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 103 out of 103 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
packages/fast-element/src/extensions/attribute-map.ts:63
- The new public
attributeMap()extension factory (and itspendingAttributeMapsside effects) is not covered by fast-element’s existing test suite. Since this is now part of the@microsoft/fast-elementpublic API surface, it would be good to add dedicated tests validating registration timing (runs duringcompose()before the returned Promise resolves) and thatTemplateElementconsumption produces the expected@attrdefinitions.
packages/fast-element/src/extensions/observer-map.ts:86 - The new public
observerMap()extension factory and its schema/config stamping logic are not covered by fast-element tests. Adding targeted tests (e.g., configpropertiesinclude/exclude behavior and$observestamping, plus timing relative tocompose()Promise resolution) would help prevent regressions now that this ships from@microsoft/fast-element.
| ): Promise<FASTElementDefinition<TType>> { | ||
| const definition = | ||
| fastElementBaseTypes.has(type) || fastElementRegistry.getByType(type) | ||
| ? new FASTElementDefinition<TType>(class extends type {}, nameOrDef) | ||
| : new FASTElementDefinition<TType>(type, nameOrDef); | ||
|
|
||
| if (extensions) { | ||
| for (const extension of extensions) { | ||
| extension(definition); | ||
| } | ||
| } | ||
|
|
||
| return Promise.resolve(definition); |
There was a problem hiding this comment.
FASTElementDefinition.compose() now executes extensions unconditionally, even if the element is already registered in the target registry. This differs from define(), which only runs extensions when !registry.get(this.name). For extensions with global side effects (e.g. observerMap()/attributeMap() populating pending registries), calling define() repeatedly or composing an already-defined element can leave stale entries and/or run side effects multiple times. Consider gating extension execution on !definition.registry.get(definition.name) (or moving extension invocation back into define() while still meeting the timing requirement), so extensions only run when a platform definition will actually occur.
|
Closing this, too messy, the code needs to be more modular before it can be brought over to fast-element. |
Pull Request
📖 Description
Moves
observerMapandattributeMapfrom@microsoft/fast-htmlto@microsoft/fast-elementasFASTElementExtensionfactory functions, building on the extensions support added in #7465.Previously these were currying functions internal to fast-html. This refactor:
FASTElementExtensionfactories in@microsoft/fast-elementextensionsparameter toFASTElementDefinition.compose()that runs extensions synchronously after definition construction (critical for timing withTemplateElementmicrotask-based registration)schemaoption andconfig:nesting"attribute-name-strategy"toattributeNameStrategy(camelCase) to fix YAML front matter parsing issues in generated API docsNew API
🎫 Issues
FASTElement.define())👩💻 Reviewer Notes
compose(), afterFASTElementDefinitionconstruction but beforePromise.resolve(). This timing is critical becauseTemplateElement.register()subscribes toisRegisteredObservable notifications and resolves promises in microtasks — extensions must populatependingObserverMaps/pendingAttributeMapsbefore any.then()handlers fire.pendingObserverMapsandpendingAttributeMapsregistries are marked@internalbut exported publicly for cross-package consumption by fast-html.📑 Test Plan
✅ Checklist
General
$ npm run change⏭ Next Steps
observerMap()andattributeMap()extension factories in fast-elementsites/website/src/docs/3.x/for the extensions API