fix: deduplicate deprecated event arg warning in fast-html template processing#7441
Merged
fix: deduplicate deprecated event arg warning in fast-html template processing#7441
Conversation
…mponent name Add a module-scoped Set to track which components have already emitted the deprecation warning for the legacy "e" event argument. The warning now fires at most once per unique component name and includes the component name in the message to help developers locate usage. Closes #7409 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ion warning Move the deprecated "e" warning from per-binding to per-template evaluation. A private _hasDeprecatedE flag is set during template processing and checked once after resolveStringsAndValues completes, emitting a single warning per f-template element. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
janechu
force-pushed
the
users/janechu/reduce-e-warnings
branch
from
db9a13a to
6277716
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces console noise in @microsoft/fast-html by deduplicating the deprecation warning for legacy e event arguments during <f-template> processing, and improves debuggability by including the component name in the warning.
Changes:
- Track deprecated
eusage during template parsing and emit a single warning after processing (instead of warning per event binding). - Add a new Playwright fixture (
deprecated-event-warning) to validate warning deduplication and message content. - Update docs (README + DESIGN) to describe the deduplicated warning behavior and component-name inclusion.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/fast-html/src/components/template.ts | Moves warning emission to a single post-processing point and includes component name in the warning. |
| packages/fast-html/test/fixtures/event/event.spec.ts | Asserts the warning message includes the component name. |
| packages/fast-html/test/fixtures/deprecated-event-warning/* | New fixture + tests validating deduplication behavior across components. |
| packages/fast-html/README.md | Documents “warn once per component” behavior and name inclusion. |
| packages/fast-html/DESIGN.md | Updates syntax constant table description for deprecated e. |
| change/@microsoft-fast-html-*.json | Beachball change entry for the fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
📖 Description
Deduplicate the deprecated
eevent argument warning in@microsoft/fast-htmltemplate processing. Previously,console.warnfired for every event binding that used the legacyesyntax across every<f-template>on the page, flooding the console with 60+ identical messages.Now the warning fires at most once per unique component name and includes the component name in the message, making it easy to locate which components still use the deprecated syntax.
🎫 Issues
👩💻 Reviewer Notes
The fix uses a module-scoped
Set<string>keyed by component name (this.name) to gate the warning. This approach was chosen over an instance-level flag because multiple<f-template>elements can share the samename, and we want exactly one warning per unique component name.📑 Test Plan
deprecated-event-warningfixture with two components (test-alphawith twoebindings,test-betawith one) to verify:eventfixture test to assert the warning includes the component name✅ Checklist
General
$ npm run change