Skip to content

fix(NG & WC): attribute passing removals - #1806

Merged
samijaber merged 4 commits into
BuilderIO:mainfrom
mfranzke:patch-3
Jul 20, 2026
Merged

fix(NG & WC): attribute passing removals#1806
samijaber merged 4 commits into
BuilderIO:mainfrom
mfranzke:patch-3

Conversation

@mfranzke

@mfranzke mfranzke commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Description

Please provide the following information:

What changes you made:

Fix (in attribute-passing.ts, 2 lines changed):

  1. const attributes = parent.attributes;const attributes = [...parent.attributes]; — creates a static snapshot before iterating
  2. const attr = attributes.item(i);const attr = attributes[i]; — uses standard array indexing on the now-static array

Test (new file attribute-passing.test.ts):

  • Uses a Proxy-based mock that faithfully simulates the live NamedNodeMap behavior (length/indexing reflect mutations in real time)
  • Evaluates the actual generated runtime code via new Function against the mock DOM
  • Verifies the exact reproduction case (data-a="1" data-b="2") plus aria, class, mixed attributes, and 3+ data attributes
  • Confirmed: the test fails with the old code (4/5 tests fail) and passes with the fix (5/5 pass)

Why you made them, and:

The enableAttributePassing runtime code iterated over parent.attributes (a live NamedNodeMap) while calling parent.removeAttribute() inside the loop. Since NamedNodeMap is a live collection, removing an attribute shrinks it mid-iteration, causing subsequent attributes to be skipped.

Any other useful context:

Make sure to follow the PR preparation steps in CONTRIBUTING.md before submitting your PR:

  • format the codebase: from the root, run yarn fmt:prettier.
  • update all snapshots (in core & CLI): from the root, run yarn test:update
  • add Changeset entry: from the root, run yarn g:changeset and follow the CLI instructions. Alternatively, use the Changeset Github Bot to create the file.

@mfranzke
mfranzke requested a review from samijaber as a code owner July 18, 2026 08:49
@changeset-bot

changeset-bot Bot commented Jul 18, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 081862c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@builder.io/mitosis Patch
@builder.io/mitosis-cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@nx-cloud

nx-cloud Bot commented Jul 18, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 0e0a1d4

Command Status Duration Result
nx run-many --target test ✅ Succeeded 4m 10s View ↗
nx run-many --target build --exclude @builder.i... ✅ Succeeded 4m 4s View ↗
nx e2e @builder.io/e2e-app ✅ Succeeded 1m 2s View ↗
nx build @builder.io/mitosis-site ✅ Succeeded 2m 40s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-20 14:54:43 UTC

@samijaber
samijaber enabled auto-merge (squash) July 20, 2026 14:53
@samijaber
samijaber merged commit da57ccd into BuilderIO:main Jul 20, 2026
7 checks passed
@mfranzke
mfranzke deleted the patch-3 branch July 20, 2026 14:57
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.

2 participants