Skip to content

Commit

Permalink
Stringify tests
Browse files Browse the repository at this point in the history
  • Loading branch information
platypii committed Dec 23, 2024
1 parent 7d3670b commit b3d1f22
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
[![minzipped](https://img.shields.io/bundlephobia/minzip/hightable)](https://www.npmjs.com/package/hightable)
[![workflow status](https://github.com/hyparam/hightable/actions/workflows/ci.yml/badge.svg)](https://github.com/hyparam/hightable/actions)
[![mit license](https://img.shields.io/badge/License-MIT-orange.svg)](https://opensource.org/licenses/MIT)
![coverage](https://img.shields.io/badge/Coverage-93-darkred)
![coverage](https://img.shields.io/badge/Coverage-94-darkred)
[![dependencies](https://img.shields.io/badge/Dependencies-0-blueviolet)](https://www.npmjs.com/package/hightable?activeTab=dependencies)

HighTable is a virtualized table component for React, designed to efficiently display large datasets in the browser. It loads and renders only the rows necessary for the current viewport, enabling smooth scrolling and performance even with millions of rows. HighTable supports asynchronous data fetching, dynamic loading, and optional column sorting.
Expand Down
1 change: 1 addition & 0 deletions src/HighTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ export default function HighTable({
export function stringify(value: any): string | undefined {
if (typeof value === 'string') return value
if (typeof value === 'number') return value.toLocaleString()
if (typeof value === 'bigint') return value.toLocaleString()

This comment has been minimized.

Copy link
@severo

severo Jan 2, 2025

Contributor

I have two comments after reading the docs for toLocaleString().

The runtime's default locale is used when undefined is passed or when none of the specified locale identifiers is supported

Is this what we want? Or do we want to force 'en-US'? (eg: running the tests on a machine with another default language would fail)

Every time toLocaleString is called, it has to perform a search in a big database of localization strings, which is potentially inefficient. When the method is called many times with the same arguments, it is better to create a Intl.NumberFormat object and use its format() method, because a NumberFormat object remembers the arguments passed to it and may decide to cache a slice of the database, so future format calls can search for localization strings within a more constrained context.

So, maybe we want to setup a NumberFormat object to (prematurely?) optimize

This comment has been minimized.

Copy link
@severo

severo Jan 2, 2025

Contributor

cc @platypii as I'm not sure you receive a notification for comments on a commit

if (Array.isArray(value)) return `[${value.map(stringify).join(', ')}]`
if (value === null || value === undefined) return JSON.stringify(value)
if (value instanceof Date) return value.toISOString()
Expand Down
50 changes: 50 additions & 0 deletions test/stringify.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { describe, expect, test } from 'vitest'
import { stringify } from '../src/HighTable.js'

describe('stringify', () => {
test('returns the same string if input is a string', () => {
expect(stringify('Hello')).toBe('Hello')
})

test('returns the localized string if input is a number', () => {
expect(stringify(4200)).toBe('4,200')
})

test('stringifies null and undefined as JSON', () => {
expect(stringify(null)).toBe('null')
expect(stringify(undefined)).toBe(undefined)
})

test('returns ISO string if input is a Date', () => {
const date = new Date('2020-01-01T00:00:00.000Z')
expect(stringify(date)).toBe('2020-01-01T00:00:00.000Z')
})

test('handles arrays', () => {
expect(stringify([1, 2, 3])).toBe('[1, 2, 3]')
expect(stringify(['a', 'b'])).toBe('[a, b]')
})

test('handles objects', () => {
const obj = { a: 1, b: 'two' }
expect(stringify(obj)).toBe('{a: 1, b: two}')
})

test('handles nested objects and arrays', () => {
const nested = {
a: [1, 2, { x: 'x' }],
b: { c: 'hello' },
}
expect(stringify(nested)).toBe('{a: [1, 2, {x: x}], b: {c: hello}}')
})

test('handles booleans', () => {
expect(stringify(true)).toBe('true')
expect(stringify(false)).toBe('false')
})

test('handles bigints', () => {
expect(stringify(BigInt(123))).toBe('123')
expect(stringify(BigInt(4200))).toBe('4,200')
})
})

0 comments on commit b3d1f22

Please sign in to comment.