Skip to content

fix: avoid rebinding attr backing fields on connect#7625

Merged
janechu merged 6 commits into
mainfrom
users/janechu/fix-7624-attr-backing-fields
Jun 26, 2026
Merged

fix: avoid rebinding attr backing fields on connect#7625
janechu merged 6 commits into
mainfrom
users/janechu/fix-7624-attr-backing-fields

Conversation

@janechu

@janechu janechu commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Pull Request

📖 Description

Fixes ElementController observable capture/rebind behavior so FAST internal backing fields, including object-valued @attr fields that have already passed through ValueConverter.fromView(), are not normalized back to public property assignments during connect().

This restores the expected behavior for converters such as JSON parsers while preserving pre-upgrade public observable capture and opt-in observer-map behavior.

🎫 Issues

👩‍💻 Reviewer Notes

Please focus review on the interaction between captureBoundObservables(), bindObservables(), AttributeDefinition.setValue(), and opt-in observerMap() proxying for public own properties.

📑 Test Plan

  • npm ci
  • npm run build:tsc -w @microsoft/fast-element
  • npm run build:rollup -w @microsoft/fast-element
  • npm run build:sizes -w @microsoft/fast-element
  • npm run doc -w @microsoft/fast-element
  • npm run doc:exports -w @microsoft/fast-element
  • npm run prebuild -w sites/website
  • npx biome check packages/fast-element/src/components/element-controller.ts packages/fast-element/src/components/element-controller.pw.spec.ts packages/fast-element/src/declarative/extension-subpaths.pw.spec.ts
  • npx playwright test --project=chromium src/components/element-controller.pw.spec.ts src/declarative/extension-subpaths.pw.spec.ts
  • npm run checkchange
  • npm run test:validation

✅ Checklist

General

  • I have included a change request file using $ npm run change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

@janechu janechu marked this pull request as ready for review June 26, 2026 18:53
@janechu

janechu commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Marking this ready for review just to run pipelines against it.

janechu and others added 5 commits June 26, 2026 12:47
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@janechu janechu force-pushed the users/janechu/fix-7624-attr-backing-fields branch from f9c6240 to 6623928 Compare June 26, 2026 19:49
@janechu janechu changed the base branch from releases/fast-element-v3 to main June 26, 2026 19:49
@janechu

janechu commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

For some strange reason the required Github Actions are not running against this PR, individual runs have been triggered.

Edit: unnecessary they are now passing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@janechu janechu merged commit 6ad856f into main Jun 26, 2026
14 checks passed
@janechu janechu deleted the users/janechu/fix-7624-attr-backing-fields branch June 26, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: ValueConverter.fromView re-invoked with already-converted value during bindObservables

3 participants