[SDK-433] JSON parse error when config contains unexpected type#858
[SDK-433] JSON parse error when config contains unexpected type#858lposen wants to merge 2 commits intoemb-ootb/masterfrom
Conversation
…tion normalization
|
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (2)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
3 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| console.warn( | ||
| `[IterableEmbeddedView] Ignoring ${String(key)}: expected number or numeric string, got ${typeof value}` | ||
| ); | ||
| return undefined; |
| next[key] = coerced; | ||
| } | ||
| } | ||
| return next; |
…r-crashes-with-incorrect-configs
There was a problem hiding this comment.
Pull request overview
Fixes crashes/JS errors when embedded view config contains unexpected types by normalizing numeric style fields before style resolution.
Changes:
- Added
normalizeEmbeddedViewConfigutility to coerce numeric config fields (e.g.,borderWidth,borderCornerRadius) from strings and drop invalid values with warnings. - Added Jest coverage for normalization behavior (coercion, warnings, non-mutation).
- Updated
useEmbeddedViewto normalize config before callinggetStyles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/embedded/utils/normalizeEmbeddedViewConfig.ts | Introduces config normalization/coercion for numeric style fields to prevent invalid types from reaching style resolution. |
| src/embedded/utils/normalizeEmbeddedViewConfig.test.ts | Adds unit tests validating coercion, dropping invalid values, warning behavior, and non-mutation. |
| src/embedded/hooks/useEmbeddedView/useEmbeddedView.ts | Wires normalization into the embedded view hook so getStyles receives sanitized config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Runtime JSON / native payloads may use strings for numeric fields. | ||
| const result = normalizeEmbeddedViewConfig(input as never); | ||
|
|
||
| expect(result).toEqual({ | ||
| borderWidth: 45, | ||
| borderCornerRadius: 12.5, | ||
| backgroundColor: '#fff', | ||
| }); | ||
| expect(warnSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('trims whitespace before parsing numeric strings', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: ' 8 ', | ||
| } as never); | ||
|
|
There was a problem hiding this comment.
The tests repeatedly cast inputs to never to bypass type checking. That makes the intent harder to follow and can hide mistakes in the test setup; prefer a more explicit cast like unknown as IterableEmbeddedViewConfig (or a small helper type) for invalid-shape inputs.
| const n = parseFloat(trimmed); | ||
| if (Number.isFinite(n)) { | ||
| return n; | ||
| } |
There was a problem hiding this comment.
parseFloat() is permissive (e.g., it will accept values like "12px" or "10abc" and return a finite number), which means malformed numeric strings won’t trigger the warning and will be applied as styles. Consider using a stricter conversion (e.g., Number(trimmed) with a full-string numeric check) so only truly numeric strings are coerced and everything else falls back to defaults.
There was a problem hiding this comment.
I thought the same as the warning above so that parseFloat("3abc") === 3 but I guess we can live with assuming people will always type 3px instead.
🔹 JIRA Ticket(s) if any
✏️ Description
Fix error when config contains unexpected type
Testing
{"borderWidth":"45"}