Skip to content

Apply convert to defaults and preserve explicit null writes#118

Merged
DmitrySharabin merged 0 commit into
fix-issue-98from
fix-defaults-convert-and-null
May 16, 2026
Merged

Apply convert to defaults and preserve explicit null writes#118
DmitrySharabin merged 0 commit into
fix-issue-98from
fix-defaults-convert-and-null

Conversation

@DmitrySharabin
Copy link
Copy Markdown
Member

@DmitrySharabin DmitrySharabin commented May 15, 2026

Summary

Two related defaults bugs in Prop.js:

  • A resolved default skipped spec.convert, so { default: 5, convert: v => v * 2 } read as 5 instead of 10. get() now applies convert after parse, matching what a user write goes through.
  • Explicit el.v = null was overridden by the default. get() no longer treats null the same as undefined. To keep removeAttribute resetting to default, set() collapses null-from-attribute to undefined.

Unlocks the two pinned tests in the Defaults group (Default passes through convert, null is preserved on a prop with a default); suite goes 93 → 95 passing.

Test plan

  • npm test — Defaults group fully green
  • Confirm reflection.js removeAttribute collapses a reflected prop to its default still passes

@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from 2b667b9 to bd68789 Compare May 15, 2026 21:34
@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from bd68789 to 2b667b9 Compare May 15, 2026 21:41
@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from 2b667b9 to 80acf81 Compare May 15, 2026 21:45
@DmitrySharabin DmitrySharabin force-pushed the fix-issue-98 branch 2 times, most recently from 787e027 to 80e7485 Compare May 15, 2026 22:03
@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from 80acf81 to 94c292e Compare May 15, 2026 22:03
@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from 94c292e to 522905e Compare May 15, 2026 22:07
@DmitrySharabin DmitrySharabin merged commit 4610bd7 into fix-issue-98 May 16, 2026
@DmitrySharabin DmitrySharabin deleted the fix-defaults-convert-and-null branch May 16, 2026 12:01
@DmitrySharabin DmitrySharabin force-pushed the fix-defaults-convert-and-null branch from 522905e to 4610bd7 Compare May 16, 2026 12:01
@DmitrySharabin
Copy link
Copy Markdown
Member Author

Note: this PR was auto-merged by GitHub as a side effect of a restack error — the head branch was force-pushed such that it became an ancestor of the base, which GitHub interprets as "already merged." The original commit 522905e was preserved by cherry-picking it onto rollback-signals as commit a599e8f, so the change is correctly landed there.

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