Uh oh!
There was an error while loading. Please reload this page.
docs(fields): record the measured browser reading behind CurrencyField/PercentField - #6777
Merged
Merged
Conversation
…d/PercentField objectui#6765 filed that both widgets hand `parseFloat(e.target.value)` to `onChange` with no whole-string guard, leaning entirely on the browser's number-input value sanitization -- and that happy-dom, this package's test environment, does not implement it. The card explicitly did NOT measure the real browser. Measured now, on Chromium 141.0.7390.37 via Playwright, driving the real widgets mounted in a real page: typed "12abc" -> box.value "12" onChange(12) typed "1.2.3" -> box.value "1.23" onChange(1.23) pasted "0x10" -> box.value "010" onChange(10) typed "1e" -> box.value "" onChange(null) validity.badInput Residue never reaches these widgets: the browser filters the keystroke or paste BEFORE the change event, so objectui#6715's anchored `WHOLE_NUMBER_TEXT` guard would accept every string these boxes can produce and reject only strings the test environment fabricates. It is deliberately not copied here, and both widgets now say so where the next reader will look. No behaviour change -- the product diff is 78 inserted comment lines and 0 deletions. `NumberInputWidgets.environmentDivergence.test.tsx` pins the oracle-vs-product disagreement (three of five readings differ) so it cannot be re-derived by accident. It asserts the unreachability through the platform's own `validity.badInput` -- the one signal happy-dom and Chromium were measured to agree on -- rather than transcribing #6715's module-private regex, which would be a second dialect of "what a number is" and would need an edit to LocationField.tsx that this card is fenced out of. Empty-frontmatter changeset: internal only, releases nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 29, 2026 11:09
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #6765
Deliberately not
Fixes: this PR carries the measurement the card asked for and thehalf of the answer that is not a judgment call. The other half — whether the silent drop
this measurement reproduced should be announced — is escalated below and needs a ruling,
so the card must stay open.
Step 1: the measurement the card explicitly did not make
#6765 measured that happy-dom, this package's test environment, does not implement the
HTML number-input value-sanitization algorithm, and concluded that any test written for
these two widgets exercises a code path no browser takes. It was explicit that it had
not measured the real browser, and had not reproduced a user-visible defect.
Measured now: Chromium 141.0.7390.37 via Playwright 1.62.1,
executablePath/opt/pw-browsers/chromium, driving the real widgets mounted from source in a realpage (a Vite harness, not committed), through every delivery route a user has —
keystrokes, Ctrl+V paste with a real clipboard on a secure origin,
keyboard.insertText,and a programmatic value set.
The real-browser table — the widgets, not a bare input
.valuebadInput12abc"12"onChange(12)12abc"12"onChange(12)1.2.3"1.23"onChange(1.23)0x10"010"onChange(10)1e""onChange(null)1e""""12abc"12"onChange(0.12)1.2.3"1.23"onChange(0.0123)0x10"010"onChange(0.1)1e""onChange(null)Programmatic sets sanitize exactly as the spec says:
el.value = "12abc"leaves"".Which fork this selects, and the correction it forces
Sanitization holds — and the card's predicted consequence does not.
The card reasoned that if sanitization held, residue would degrade to the empty string and
onChange(null). Measured,12abc/1.2.3/0x10never take that route. The browserfilters the keystroke or the paste before the change event fires, so what arrives is a
different, already well-formed number:
"12","1.23","010".The load-bearing consequence: residue never reaches these two widgets in a real
browser.#6715's anchored
WHOLE_NUMBER_TEXT— the right answer forLocationField,a
type="text"box with nothing filtering it — would here accept every string these boxescan produce and reject only strings the test environment fabricates. Adding it would be a
no-op in the product whose only observable effect is making a happy-dom test go green over
a branch no user executes, and it would pin a truncation nobody reaches. That is the
outcome the dispatch fenced off, so it is not added.
The oracle really does disagree with the product
12abc(currency)12121.2.3(currency)1.21.230x10(currency)0101e(currency)1null1.2.3(percent)0.0120.01230x10(percent)00.11e(percent)0.01nullSix of ten cases disagree. A pin written in this environment asserting
0x10yields0would assert the exact opposite of the product, which yields
10.What is in this PR
CurrencyField.tsx,PercentField.tsx— the measurement recorded at theparseFloatsites, with the explicit note that finding(fields):
LocationFieldaccepts a partly-numeric coordinate, emitting a plausible wrong location #6715's guard is deliberately not copied and why.Comment-only: 78 inserted lines, 0 deletions, and every inserted line is a comment
(
git diff -U0 | grep -v '^+\s*\(\*\|//\|/\*\*\)'is empty). Behaviour unchanged.NumberInputWidgets.environmentDivergence.test.tsx— 16 tests pinning the divergence:that happy-dom does not sanitize, that it nevertheless reports
validity.badInputcorrectly, that the platform already calls every measured browser reading a whole number
and every residue string bad input, and the per-widget oracle-vs-product table above.
first-class way to declare that; the
skip-changesetlabel is deliberately not usedhere, it reads nothing in this repo).
The unreachability assertion uses the platform's own
validity.badInputrather than atranscription of #6715's regex. That regex is module-private to
LocationField.tsx, thiscard is fenced out of editing that file to export it, and copying its source would create
the second dialect of "what a number is" AGENTS.md #0.1 forbids.
badInputis also theone signal on this surface that happy-dom and Chromium were measured to agree on.
Escalated, not fixed: the silent drop is real
The card listed "whether a silent drop is worth announcing" as unmeasured. The drop itself
is now reproduced: typing
1eleaves Chromium visibly displaying1ewhile.valuereads""; the widget emitsnull,aria-invalidstays"false", and zerodiagnostics render. Pasting
1einto an empty box is worse — React sees no value changeand fires nothing at all, so the model keeps its previous value while the box shows text
that contradicts it. That is #6716's class.
It is not fixed here, for two measured reasons rather than as a scope dodge:
type="number"widget in this package —NumberFieldandGeolocationFieldread the box the same way. Announcing in two offour is a widget-class decision, not a patch.
1.2.3becoming1.23) cannot be refused by any widget-sideguard at all — the information is gone before
handleChangeruns. Announcing onbadInputwhile1.2.3still stores silently teaches users that no warning means thevalue is right, which is exactly when it is not. Refusing those too means abandoning
type="number"fortype="text"plus own parsing, which reverses Record-level inline edit polish (follow-up to #2407): expanded-value passthrough, approval-lock preflight, numeric inputs, edit-CTA state, keyboard shortcuts #2572's deliberatemin/max/stepaffordances.Options and a recommendation are in the report on #6765.
Verification
All heavy runs serialized through the shared verify lock. Union re-run on the shipping
commit
816e3b824, chained with&&so the wrapper verdict certifies every part.pnpm exec vitest run packages/fields/Test Files 119 passed (119)·Tests 1972 passed (1972)pnpm exec vitest run packages/fields/src/__tests__/NumberInputWidgets.environmentDivergence.test.tsxTests 16 passed (16)tsc --noEmit+tsc -p tsconfig.test.json(packages/fields)pnpm run check:control-bytescheck-control-bytes: OK (scanned 5631 tracked text file(s))pnpm run check:vi-mock-specifierscheck-vi-mock-specifiers: OK (3953 tracked source file(s))node scripts/check-lint-coverage.mjslint coverage: 46/46 packages linted, 0 with outstanding errors (0 total)node scripts/check-changeset-presence.mjs3 source file(s) ... declares 1 changeset(s)(empty frontmatter accepted)node scripts/check-changeset-no-major.mjsNo changeset declares a major bumpThe dependency closure was built first (
pnpm --workspace-concurrency=2 --filter "@object-ui/fields^..." build) — without ittscreports 100+TS2307 Cannot find moduleerrors that are a fresh-worktree prerequisite failure, not a red gate.
Ablation, run on this commit to show the new pin measures the widget rather than
restating a constant table: swapping
parseFloatforNumberinCurrencyField's changehandler. Mutation confirmed on disk by grep counts before reading anything
(
parseFloat-form 1 to 0,Number-form 0 to 1); no rebuild needed and none skipped,because the test imports
../widgets/CurrencyField— a relative source import, with nodist/in the resolution path. Inner vitest exit1, red on exactly the four currencycases:
expected NaN to be 12,expected NaN to be 1.2,expected 16 to be +0(theNumber('0x10')reading #6715 rejected on measurement),expected NaN to be 1. Percentuntouched, as predicted. Restored under a
trap ... EXIT INT TERMwith absolute paths viagit checkout HEAD -- "$REPO_ROOT/...", and the restore proven by an emptygit diff HEAD, not by an exit code.Generated by Claude Code
Generated by Claude Code