Skip to content

[LiveComponent] Upgrade Idiomorph to 0.7.4 - #3868

Open
smnandre wants to merge 1 commit into
symfony:3.xfrom
smnandre:sa/live-idiomorph-upgrade
Open

smnandre wants to merge 1 commit into
symfony:3.xfrom
smnandre:sa/live-idiomorph-upgrade

Conversation

@smnandre

Copy link
Copy Markdown
Member
Q A
Bug fix? no
New feature? no
Deprecations? no
Documentation? no
Issues Fix #...
License MIT

This one is tricky and would need a lot of manual tests by users... it may well fix a lot of small untracable bugs about focus and ids..... but may also trigger some changes userland, I'm not really sure.

But we need to upgrade to then leverage some other small improvmnts in idiomorph.

--

Improves focus restoration and keyed DOM moves while preserving external widgets and client-side ID changes.

Update docs: changing a server-rendered ID recreates the element/component.

@github-actions

Copy link
Copy Markdown
Contributor

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

FileBefore (Size / Gzip)After (Size / Gzip)
LiveComponent
live_controller.js 85.57 kB / 19.17 kB 89.9 kB+5% 📈 / 20.29 kB+6% 📈

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Parent renders can fail after JavaScript changes a child component’s root ID because preserved-child lookup cannot recover its server ID.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Upgrades LiveComponent to Idiomorph 0.7.4 and adapts DOM morphing to preserve client mutations, focus, and keyed elements.

Changes:

  • Tracks original server IDs during client-side mutations.
  • Updates morphing behavior for keyed moves and external widgets.
  • Adds documentation and broad unit/browser coverage.
File summaries
File Description
bin/build_package.ts Safely removes standalone JSDoc during builds.
pnpm-lock.yaml Locks Idiomorph 0.7.4.
src/LiveComponent/assets/package.json Upgrades Idiomorph.
src/LiveComponent/assets/dist/live_controller.js Updates the compiled distribution.
src/LiveComponent/assets/src/Rendering/ExternalMutationTracker.ts Tracks original server IDs.
src/LiveComponent/assets/src/morphdom.ts Integrates new morphing and ID behavior.
src/LiveComponent/assets/test/browser/live-component.test.ts Adds browser regressions.
src/LiveComponent/assets/test/unit/Component/index.test.ts Documents an intentional lint exception.
src/LiveComponent/assets/test/unit/Rendering/ExternalMutationTracker.test.ts Tests ID tracking lifecycle.
src/LiveComponent/assets/test/unit/controller/action.test.ts Removes an unused variable.
src/LiveComponent/assets/test/unit/controller/child.test.ts Verifies child replacement by ID.
src/LiveComponent/assets/test/unit/controller/render-with-external-changes.test.ts Covers external DOM mutations and keyed moves.
src/LiveComponent/assets/test/unit/morphdom.test.ts Tests ID restoration after failures.
src/LiveComponent/doc/index.rst Documents server and client ID behavior.
Review details

Files not reviewed (1)

  • pnpm-lock.yaml: Generated file
  • Files reviewed: 11/14 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +94 to +96
const oldElement = rootFromElement.querySelector(`#${id}`);
if (!(oldElement instanceof HTMLElement)) {
throw new Error(`The element with id "${id}" was not found in the original HTML`);

@Kocal Kocal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI is not happy, would you mind fixing these issues before I review? Thanks!

@smnandre

Copy link
Copy Markdown
Member Author

Not sure i will soon to be honest, most are for calendar links here... and I got bad exp with CI lately :)

(By soon I mean in the next days)

Will do later this week, but other/people with issues about these problèms with LiveComponent can start looking/testing

@Kocal

Kocal commented Sep 13, 2026

Copy link
Copy Markdown
Member

Not sure i will soon to be honest, most are for calendar links here... and I got bad exp with CI lately :)

You got tricked as well, but nope, it's a bug from pnpm when running pnpm run test:browser (pnpm run -r --workspace-concurrency=1 test:browser), it execute pnpm run test:browser for the next package, when tests from the previous package fail.

Here, it stopped at CalendarLink, but the actual fails are coming from Autocomplete:

    1) [chrome-latest] › test/browser/dynamic-form.test.ts:24:5 › Autocomplete with Dynamic Forms › should not throw "Tom Select already initialized" error when switching between dynamic autocomplete fields 
  
      Error: expect(locator).toHaveCount(expected) failed
  
      Locator:  locator('[data-test-id="autocomplete-option"]')
      Expected: 1
      Received: 0
      Timeout:  5000ms
  
      Call log:
        - Expect "toHaveCount" with timeout 5000ms
        - waiting for locator('[data-test-id="autocomplete-option"]')
          9 × locator resolved to 0 elements
            - unexpected value "0"
  
  
        65 |         expect(tomSelectError).toBeUndefined();
        66 |
      > 67 |         await expect(page.locator('[data-test-id="autocomplete-option"]')).toHaveCount(1);
           |                                                                            ^
        68 |     });
        69 |
        70 |     test('should properly disconnect and reconnect Tom Select on rapid type changes', async ({ page }) => {
          at /home/runner/work/ux/ux/src/Autocomplete/assets/test/browser/dynamic-form.test.ts:67:76

I'm not sure why it happens tho

@smnandre

Copy link
Copy Markdown
Member Author

I think there could be (on the autocomplete case) some timing issue (we had that problem for years now.. whatever CI / testing framework we use).

I'll look at the other things this we :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants