Add focused Morphlex regression coverage#44
Conversation
Protect detached-node callback behavior and no-op character data semantics while bringing src/morphlex.ts to full coverage with focused browser-first tests.
Verify realistic app-shell and full-document morphs with HTML fixtures so large structural updates stay stable across browser environments.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62e2cdaf90
ℹ️ 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".
| const parent = from.parentNode | ||
|
|
||
| if (!parent) { | ||
| for (let i = 0; i < newNodes.length; i++) { |
There was a problem hiding this comment.
In the length > 1 path, #morphOneToOne(from, first) runs before parent is captured, so when the first target requires replacement (for example, morphing an attached <span> to "<div>...</div><em>...</em>"), from.parentNode is already null here and this branch returns early as if the source were detached. That drops all trailing nodes from the target list, so attached one-to-many morphs can produce incomplete DOM output.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds focused regression tests for the Morphlex DOM morphing library, targeting specific edge cases to achieve full coverage of src/morphlex.ts. Alongside the new tests, it includes several small source refactors to make those code paths easier to reach and verify.
Changes:
- Adds 15+ new browser-first test files covering detached-node callbacks, character-data no-op semantics, whitespace handling,
morphInnervalidation,id-array matching, and form control dirty-flagging. - Refactors
src/morphlex.tsto clarify detached-node behavior in#replaceNodeand#morphOneToMany, simplifiesflagDirtyInputs, removes provably-redundant active-flag guards in the first matching passes, and simplifiesforEachDescendantElementWithId. - Adds a
coverage.includeconstraint tovitest.config.tsand three.gitignoreentries for coverage temp files.
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/morphlex.ts |
Core refactors: detached-node callback semantics, flagDirtyInputs signature, redundant guard removal, longestIncreasingSubsequence simplification |
vitest.config.ts |
Adds coverage.include scoped to src/morphlex.ts |
.gitignore |
Ignores coverage temp directories |
test/new/before-node-added.browser.test.ts |
New tests for detached-node beforeNodeAdded/beforeNodeRemoved behavior |
test/new/callback-guards.browser.test.ts |
New tests for beforeNodeVisited, afterNodeVisited, beforeChildrenVisited guards |
test/new/character-data.browser.test.ts |
New tests verifying no-op behavior when character data is unchanged |
test/new/dirty-option.browser.test.ts |
New tests for option dirty-flagging before morph |
test/new/dirty-textarea.browser.test.ts |
New tests for descendant textarea dirty-flag cleanup |
test/new/document-target.browser.test.ts |
New test for morphDocument accepting a pre-parsed document |
test/new/document.browser.test.ts |
New test for id-based matching from the document root |
test/new/duplicate-id.browser.test.ts |
New test for bucketed duplicate-id candidate matching |
test/new/id-array.browser.test.ts |
New tests for idArray/idSet descendant matching logic |
test/new/morph-inner.browser.test.ts |
New tests for morphInner string/mismatch validation |
test/new/no-matches.browser.test.ts |
New test for completely unmatched child removal and insertion |
test/new/non-element-matching.browser.test.ts |
New tests for comment/text node matching fallback |
test/new/textarea.browser.test.ts |
New tests for textarea morph including preserveChanges |
test/new/whitespace.browser.test.ts |
Two new whitespace edge-case tests (empty and unicode) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 30 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { expect, test } from "vitest" | ||
| import { morph, morphInner } from "../../src/morphlex" | ||
|
|
||
| test("beforeNodeVisited can skip non-element updates without firing afterNodeVisited", () => { |
There was a problem hiding this comment.
The test name says "without firing afterNodeVisited" but the test expects events to equal ["before", "after"], which includes "after" — the afterNodeVisited callback for the outer div element. The name is misleading: while it's true that afterNodeVisited is not fired for the text node itself (because beforeNodeVisited returned false for it), the afterNodeVisited callback does fire for the surrounding div. A clearer name would be something like "beforeNodeVisited returning false for a text node skips afterNodeVisited for that node but not the parent element".
Protect detached-node callback behavior and no-op character data semantics while bringing src/morphlex.ts to full coverage with focused browser-first tests.