-
Notifications
You must be signed in to change notification settings - Fork 44
[SDK-433] JSON parse error when config contains unexpected type #858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: emb-ootb/master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| import { normalizeEmbeddedViewConfig } from './normalizeEmbeddedViewConfig'; | ||
|
|
||
| describe('normalizeEmbeddedViewConfig', () => { | ||
| let warnSpy: jest.SpyInstance; | ||
|
|
||
| beforeEach(() => { | ||
| warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| warnSpy.mockRestore(); | ||
| }); | ||
|
|
||
| it('returns null or undefined unchanged', () => { | ||
| expect(normalizeEmbeddedViewConfig(null)).toBeNull(); | ||
| expect(normalizeEmbeddedViewConfig(undefined)).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('parses numeric strings for borderWidth and borderCornerRadius', () => { | ||
| const input = { | ||
| borderWidth: '45', | ||
| borderCornerRadius: '12.5', | ||
| backgroundColor: '#fff', | ||
| }; | ||
|
|
||
| // 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); | ||
|
|
||
| expect(result?.borderWidth).toBe(8); | ||
| expect(warnSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('leaves valid numbers unchanged', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: 3, | ||
| borderCornerRadius: 0, | ||
| }); | ||
|
|
||
| expect(result?.borderWidth).toBe(3); | ||
| expect(result?.borderCornerRadius).toBe(0); | ||
| expect(warnSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('drops non-parsable strings and warns', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: 'nope', | ||
| borderCornerRadius: '10', | ||
| } as never); | ||
|
|
||
| expect(result?.borderWidth).toBeUndefined(); | ||
| expect(result?.borderCornerRadius).toBe(10); | ||
| expect(warnSpy).toHaveBeenCalledTimes(1); | ||
| expect(warnSpy.mock.calls[0][0]).toContain('borderWidth'); | ||
| }); | ||
|
|
||
| it('drops empty strings and warns', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: ' ', | ||
| } as never); | ||
|
|
||
| expect(result?.borderWidth).toBeUndefined(); | ||
| expect(warnSpy).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('drops NaN and Infinity numbers and warns', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: Number.NaN, | ||
| borderCornerRadius: Number.POSITIVE_INFINITY, | ||
| } as never); | ||
|
|
||
| expect(result?.borderWidth).toBeUndefined(); | ||
| expect(result?.borderCornerRadius).toBeUndefined(); | ||
| expect(warnSpy).toHaveBeenCalledTimes(2); | ||
| }); | ||
|
|
||
| it('drops invalid types and warns', () => { | ||
| const result = normalizeEmbeddedViewConfig({ | ||
| borderWidth: true, | ||
| } as never); | ||
|
|
||
| expect(result?.borderWidth).toBeUndefined(); | ||
| expect(warnSpy).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not mutate the original config object', () => { | ||
| const original = { borderWidth: '7' as const }; | ||
| const snapshot = { ...original }; | ||
|
|
||
| normalizeEmbeddedViewConfig(original as never); | ||
|
|
||
| expect(original).toEqual(snapshot); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| import type { IterableEmbeddedViewConfig } from '../types/IterableEmbeddedViewConfig'; | ||
|
|
||
| const NUMERIC_KEYS: (keyof Pick< | ||
| IterableEmbeddedViewConfig, | ||
| 'borderWidth' | 'borderCornerRadius' | ||
| >)[] = ['borderWidth', 'borderCornerRadius']; | ||
|
|
||
| function coerceNumericField( | ||
| key: 'borderWidth' | 'borderCornerRadius', | ||
| value: unknown | ||
| ): number | undefined { | ||
| if (value === undefined || value === null) { | ||
| return undefined; | ||
| } | ||
| if (typeof value === 'number') { | ||
| if (Number.isFinite(value)) { | ||
| return value; | ||
| } | ||
| console.warn( | ||
| `[IterableEmbeddedView] Ignoring ${String(key)}: expected a finite number, got ${String(value)}` | ||
| ); | ||
| return undefined; | ||
| } | ||
| if (typeof value === 'string') { | ||
| const trimmed = value.trim(); | ||
| if (trimmed === '') { | ||
| console.warn( | ||
| `[IterableEmbeddedView] Ignoring ${String(key)}: empty string is not a valid number` | ||
| ); | ||
| return undefined; | ||
| } | ||
| const n = parseFloat(trimmed); | ||
| if (Number.isFinite(n)) { | ||
| return n; | ||
| } | ||
|
Comment on lines
+32
to
+35
|
||
| console.warn( | ||
| `[IterableEmbeddedView] Ignoring ${String(key)}: could not parse string as a number: ${JSON.stringify(value)}` | ||
| ); | ||
| return undefined; | ||
| } | ||
| console.warn( | ||
| `[IterableEmbeddedView] Ignoring ${String(key)}: expected number or numeric string, got ${typeof value}` | ||
| ); | ||
| return undefined; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
|
|
||
| /** | ||
| * Returns a shallow copy of config with numeric fields coerced from strings when possible. | ||
| * Values that cannot be coerced are omitted so style resolution can fall back to defaults. | ||
| */ | ||
| export function normalizeEmbeddedViewConfig( | ||
| config: IterableEmbeddedViewConfig | null | undefined | ||
| ): IterableEmbeddedViewConfig | null | undefined { | ||
| if (config == null) { | ||
| return config; | ||
| } | ||
| const next: IterableEmbeddedViewConfig = { ...config }; | ||
| const loose = config as Record<string, unknown>; | ||
| for (const key of NUMERIC_KEYS) { | ||
| const raw = loose[key as string]; | ||
| if (raw === undefined) { | ||
| continue; | ||
| } | ||
| if (typeof raw === 'number' && Number.isFinite(raw)) { | ||
| continue; | ||
| } | ||
| const coerced = coerceNumericField(key, raw); | ||
| if (coerced === undefined) { | ||
| delete next[key]; | ||
| } else { | ||
| next[key] = coerced; | ||
| } | ||
| } | ||
| return next; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests repeatedly cast inputs to
neverto bypass type checking. That makes the intent harder to follow and can hide mistakes in the test setup; prefer a more explicit cast likeunknown as IterableEmbeddedViewConfig(or a small helper type) for invalid-shape inputs.