Restrict tag-name matching to exclude elements with identity or form state#42
Restrict tag-name matching to exclude elements with identity or form state#42joeldrapper merged 1 commit intomainfrom
Conversation
…state The tag-name fallback pass now skips elements that have distinguishing attributes (id, name, href, src, descendant IDs) or are form controls (input, textarea, select, form-associated custom elements). These elements carry identity or live state that shouldn't be transferred to an unrelated element that happens to share the same tag name.
There was a problem hiding this comment.
Pull request overview
This PR tightens Morphlex’s tag-name fallback matching to avoid reusing elements that carry identity or live form state, preventing state/identity from being transferred onto unrelated same-tag elements during morphing.
Changes:
- Restrict tag-name fallback matching to elements without
id,name/href/src, or descendant IDs, and exclude form controls (including form-associated custom elements). - Add
isFormControl()helper and apply the new exclusions on both the “to” element and “from” candidates in the tag-name fallback pass. - Add/adjust browser and coverage tests to validate the new matching behavior (and update expectations for inputs/textareas accordingly).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/morphlex.ts |
Updates the tag-name fallback matching pass to skip identity/stateful elements and introduces isFormControl(). |
test/new/tagname-exclusions.browser.test.ts |
Adds targeted regression tests covering the new exclusions (id/name/href/src/descendant IDs/form controls/custom elements). |
test/morphlex-coverage.test.ts |
Adjusts a coverage test to ensure the input is still matched (via name) so property updates are validated on the reused element. |
test/ai-gen-coverage/localname-matching.browser.test.ts |
Updates expectations to reflect that bare inputs are replaced rather than reused by fallback matching. |
test/ai-gen-coverage/input-type-mismatch.browser.test.ts |
Updates test names/expectations for bare input replacement behavior. |
test/ai-gen-coverage/input-type-continue.browser.test.ts |
Updates a test to reflect that bare inputs are replaced (not reused) by fallback matching. |
test/ai-gen-coverage/input-localname-matching.browser.test.ts |
Updates expectations to reflect bare input replacement behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe("Property updates", () => { | ||
| it("should update input disabled property", () => { | ||
| const parent = document.createElement("div") | ||
| const input = document.createElement("input") | ||
| input.disabled = false | ||
| parent.appendChild(input) | ||
|
|
||
| const reference = document.createElement("div") | ||
| const refInput = document.createElement("input") | ||
| refInput.disabled = true | ||
| reference.appendChild(refInput) | ||
|
|
||
| morph(parent, reference) | ||
|
|
||
| expect(input.disabled).toBe(true) | ||
| }) | ||
| it("should update input disabled property", () => { | ||
| const parent = document.createElement("div") | ||
| const input = document.createElement("input") | ||
| input.disabled = false |
There was a problem hiding this comment.
Indentation in this describe("Property updates") section is inconsistent with the rest of the file (the it(...) is not indented under the describe, and its body follows the same). Please reformat (e.g., run Prettier) so the nesting matches the surrounding tests.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab6ba02298
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if ( | ||
| element.id !== "" || | ||
| isFormControl(element) || |
There was a problem hiding this comment.
Keep unkeyed dirty form controls matchable with preserveChanges
With preserveChanges: true, dirty form controls are marked before morphing, which prevents the isEqualNode fast-path and pushes matching to later passes. This new unconditional isFormControl(element) filter removes all unkeyed input/textarea/select elements from tag-name fallback, so controls without id/name are now replaced instead of reused, dropping user-entered value and focus even when the control is logically the same node. This is a behavior regression for the preserve-changes path and should be conditioned so form state is not discarded in common unkeyed forms.
Useful? React with 👍 / 👎.
The tag-name fallback pass now skips elements that have distinguishing attributes (id, name, href, src, descendant IDs) or are form controls (input, textarea, select, form-associated custom elements). These elements carry identity or live state that shouldn't be transferred to an unrelated element that happens to share the same tag name.