Skip to content

Commit 81fdaf3

Browse files
authored
JSON heuristic: don't consider strings as primitive (#340)
* Don't consider strings as primitive * fix tests * adapt text
1 parent 779314b commit 81fdaf3

File tree

2 files changed

+18
-10
lines changed

2 files changed

+18
-10
lines changed

src/components/Json/Json.test.tsx

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,10 @@ describe('Json Component', () => {
2222
})
2323

2424
it.for([
25-
['foo', 'bar'],
2625
[],
2726
[1, 2, 3],
28-
[1, 'foo', null],
2927
Array.from({ length: 101 }, (_, i) => i),
30-
])('collapses any array', (array) => {
28+
])('collapses any array with primitives', (array) => {
3129
const { getByRole } = render(<Json json={array} />)
3230
expect(getByRole('treeitem').ariaExpanded).toBe('false')
3331
})
@@ -68,14 +66,22 @@ describe('Json Component', () => {
6866
it.for([
6967
[1, 'foo', [1, 2, 3]],
7068
[1, 'foo', { nested: true }],
71-
])('expands short arrays with non-primitive values', (arr) => {
69+
])('expands short arrays with inner arrays or objects', (arr) => {
7270
const { getAllByRole } = render(<Json json={arr} />)
7371
const treeItems = getAllByRole('treeitem')
7472
expect(treeItems.length).toBe(2)
7573
expect(treeItems[0]?.getAttribute('aria-expanded')).toBe('true') // the root
7674
expect(treeItems[1]?.getAttribute('aria-expanded')).toBe('false') // the non-primitive value (object/array)
7775
})
7876

77+
it.for([
78+
['foo', 'bar'],
79+
[1, 'foo', null],
80+
])('expands short arrays with strings', (arr) => {
81+
const { getByRole } = render(<Json json={arr} />)
82+
expect(getByRole('treeitem').getAttribute('aria-expanded')).toBe('true')
83+
})
84+
7985
it.for([
8086
{ obj: [314, null] },
8187
{ obj: { nested: true } },
@@ -147,25 +153,26 @@ describe('Json Component', () => {
147153
})
148154

149155
describe('isPrimitive', () => {
150-
it('returns true only for primitive types', () => {
151-
expect(isPrimitive('test')).toBe(true)
156+
it('returns true only for primitive types (string is an exception)', () => {
152157
expect(isPrimitive(42)).toBe(true)
153158
expect(isPrimitive(true)).toBe(true)
154159
expect(isPrimitive(1n)).toBe(true)
155160
expect(isPrimitive(null)).toBe(true)
156161
expect(isPrimitive(undefined)).toBe(true)
157162
expect(isPrimitive({})).toBe(false)
158163
expect(isPrimitive([])).toBe(false)
164+
expect(isPrimitive('test')).toBe(false)
159165
})
160166
})
161167

162168
describe('shouldObjectCollapse', () => {
163-
it('returns true for objects with all primitive values', () => {
164-
expect(shouldObjectCollapse({ a: 1, b: 'test' })).toBe(true)
169+
it('returns true for objects with all primitive (but string) values', () => {
170+
expect(shouldObjectCollapse({ a: 1, b: false })).toBe(true)
165171
})
166172

167-
it('returns false for objects with non-primitive values', () => {
173+
it('returns false for objects with non-primitive (or string) values', () => {
168174
expect(shouldObjectCollapse({ a: 1, b: {} })).toBe(false)
175+
expect(shouldObjectCollapse({ a: 1, b: 'test' })).toBe(false)
169176
})
170177

171178
it('returns true for large objects', () => {

src/components/Json/helpers.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ export function isPrimitive(value: unknown): boolean {
44
value === null ||
55
!Array.isArray(value) &&
66
typeof value !== 'object' &&
7-
typeof value !== 'function'
7+
typeof value !== 'function' &&
8+
typeof value !== 'string' // exception: don't consider strings as primitive here
89
)
910
}
1011

0 commit comments

Comments
 (0)