WEBDEV-8711 add type check and value conversion at the top of the function - #71
WEBDEV-8711 add type check and value conversion at the top of the function#71dualcnhq wants to merge 1 commit into
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 78.62% 78.51% -0.12%
==========================================
Files 17 17
Lines 697 698 +1
Branches 189 190 +1
==========================================
Hits 548 548
Misses 100 100
- Partials 49 50 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| * Submits the result if enough characters are filled. | ||
| */ | ||
| private fillInputs(value: string) { | ||
| if (typeof value !== 'string') value = String(value ?? ''); |
There was a problem hiding this comment.
This coerces anything, not just numbers. numericOnly defaults to true so the regex drops the garbage and we're fine in practice, but at numericOnly=false an object prefill becomes "[object Object]" and fills the boxes with objectO. Bailing reads safer than coercing for anything that isn't a string or a number:
if (typeof value === 'number') value = String(value);
if (typeof value !== 'string') return;Also String(value ?? '') turns a null prefill into '', which then falls into clearInputs(). Probably what you want, just flagging it's a behavior change.
Q: was the number case the actual intent here, or just the easiest thing to test?
| expect(inputs?.[5].value).to.equal(''); | ||
| }); | ||
|
|
||
| test('does not throw if prefill value property is set to a non-string value', async () => { |
There was a problem hiding this comment.
This covers the one path that never crashed. The reported error is handleInput reading e.data off beforeinput, not prefillValue. If we're hardening against a non-string reaching fillInputs, worth firing a beforeinput with a non-string data too:
const ev = new InputEvent('beforeinput', { bubbles: true });
Object.defineProperty(ev, 'data', { value: {} });
inputs?.[0].dispatchEvent(ev);
|
Left two inline notes. Bigger picture though: I don't think this error is ours. Wrote it up on WEBDEV-8711 with the detail. Short version: Happy to take this as hardening on its own merits, I just don't want it closing the ticket. |
3 participants
No description provided.