Skip to content

Commit

Permalink
chore: fix not always remembering the initial enforceString state
Browse files Browse the repository at this point in the history
  • Loading branch information
josdejong committed Jul 26, 2024
1 parent bc52545 commit 2ee127b
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 54 deletions.
62 changes: 31 additions & 31 deletions src/lib/components/__snapshots__/JSONEditor.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[`JSONEditor > render table mode 1`] = `
class="jse-main svelte-57bmz4"
>
<div
class="jse-table-mode svelte-gl60hz"
class="jse-table-mode svelte-u14cgx"
role="table"
>
<div
Expand Down Expand Up @@ -209,38 +209,38 @@ exports[`JSONEditor > render table mode 1`] = `
</div>
<label
class="jse-hidden-input-label svelte-gl60hz"
class="jse-hidden-input-label svelte-u14cgx"
>
<input
class="jse-hidden-input svelte-gl60hz"
class="jse-hidden-input svelte-u14cgx"
readonly=""
tabindex="-1"
type="text"
/>
</label>
<div
class="jse-search-box-container svelte-gl60hz"
class="jse-search-box-container svelte-u14cgx"
>
</div>
<div
class="jse-contents svelte-gl60hz"
class="jse-contents svelte-u14cgx"
>
<table
class="jse-table-main svelte-gl60hz"
class="jse-table-main svelte-u14cgx"
>
<tbody>
<tr
class="jse-table-row jse-table-row-header svelte-gl60hz"
class="jse-table-row jse-table-row-header svelte-u14cgx"
>
<th
class="jse-table-cell jse-table-cell-header svelte-gl60hz"
class="jse-table-cell jse-table-cell-header svelte-u14cgx"
/>
<th
class="jse-table-cell jse-table-cell-header svelte-gl60hz"
class="jse-table-cell jse-table-cell-header svelte-u14cgx"
>
<button
class="jse-column-header svelte-2i3vdx"
Expand All @@ -256,7 +256,7 @@ exports[`JSONEditor > render table mode 1`] = `
</button>
</th>
<th
class="jse-table-cell jse-table-cell-header svelte-gl60hz"
class="jse-table-cell jse-table-cell-header svelte-u14cgx"
>
<button
class="jse-column-header svelte-2i3vdx"
Expand All @@ -275,31 +275,31 @@ exports[`JSONEditor > render table mode 1`] = `
</tr>
<tr
class="jse-table-invisible-start-section"
class="jse-table-invisible-start-section svelte-u14cgx"
>
<td
class="svelte-gl60hz"
class="svelte-u14cgx"
colspan="2"
style="height: 0px;"
/>
</tr>
<tr
class="jse-table-row svelte-gl60hz"
class="jse-table-row svelte-u14cgx"
>
<th
class="jse-table-cell jse-table-cell-gutter svelte-gl60hz"
class="jse-table-cell jse-table-cell-gutter svelte-u14cgx"
>
0
</th>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F0%2Fid"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-number jse-table-cell svelte-9ohlz8"
Expand All @@ -318,11 +318,11 @@ exports[`JSONEditor > render table mode 1`] = `
</td>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F0%2Fname"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-string jse-empty jse-table-cell svelte-9ohlz8"
Expand All @@ -343,21 +343,21 @@ exports[`JSONEditor > render table mode 1`] = `
</tr>
<tr
class="jse-table-row svelte-gl60hz"
class="jse-table-row svelte-u14cgx"
>
<th
class="jse-table-cell jse-table-cell-gutter svelte-gl60hz"
class="jse-table-cell jse-table-cell-gutter svelte-u14cgx"
>
1
</th>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F1%2Fid"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-number jse-table-cell svelte-9ohlz8"
Expand All @@ -376,11 +376,11 @@ exports[`JSONEditor > render table mode 1`] = `
</td>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F1%2Fname"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-string jse-table-cell svelte-9ohlz8"
Expand All @@ -401,21 +401,21 @@ exports[`JSONEditor > render table mode 1`] = `
</tr>
<tr
class="jse-table-row svelte-gl60hz"
class="jse-table-row svelte-u14cgx"
>
<th
class="jse-table-cell jse-table-cell-gutter svelte-gl60hz"
class="jse-table-cell jse-table-cell-gutter svelte-u14cgx"
>
2
</th>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F2%2Fid"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-number jse-table-cell svelte-9ohlz8"
Expand All @@ -434,11 +434,11 @@ exports[`JSONEditor > render table mode 1`] = `
</td>
<td
class="jse-table-cell svelte-gl60hz"
class="jse-table-cell svelte-u14cgx"
data-path="%2F2%2Fname"
>
<div
class="jse-value-outer svelte-gl60hz"
class="jse-value-outer svelte-u14cgx"
>
<div
class="jse-value jse-string jse-empty jse-table-cell svelte-9ohlz8"
Expand All @@ -463,7 +463,7 @@ exports[`JSONEditor > render table mode 1`] = `
class="jse-table-invisible-end-section"
>
<td
class="svelte-gl60hz"
class="svelte-u14cgx"
colspan="2"
style="height: 0px;"
/>
Expand Down
29 changes: 26 additions & 3 deletions src/lib/logic/documentState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
type DocumentState,
type ObjectDocumentState,
type OnExpand,
type ValueDocumentState,
type VisibleSection
} from '$lib/types.js'
import { deleteIn, getIn, type JSONPatchDocument, setIn, updateIn } from 'immutable-json-patch'
Expand Down Expand Up @@ -859,14 +860,36 @@ describe('documentState', () => {
)
})

test('should keep/update enforce string', () => {
test('should determine enforce string', () => {
const json1 = 42
const documentState1 = createDocumentState({ json: json1 })
assert.strictEqual(getEnforceString(json1, documentState1, [], JSON), false)
assert.strictEqual(getEnforceString(json1, documentState1, []), false)

const json2 = '42'
const documentState2 = createDocumentState({ json: json2 })
assert.strictEqual(getEnforceString(json2, documentState2, [], JSON), true)
assert.strictEqual(getEnforceString(json2, documentState2, []), true)

const json3 = 'true'
const documentState3 = createDocumentState({ json: json3 })
assert.strictEqual(getEnforceString(json3, documentState3, []), true)

const json4 = 'null'
const documentState4 = createDocumentState({ json: json4 })
assert.strictEqual(getEnforceString(json4, documentState4, []), true)
})

test('should create enforce string state if needed', () => {
const json = '42'
const documentState = createDocumentState({ json })
assert.strictEqual(getEnforceString(json, documentState, []), true)
assert.strictEqual((documentState as ValueDocumentState).enforceString, undefined)

const result = documentStatePatch(json, documentState, [
{ op: 'replace', path: '', value: 'abc' }
])
assert.strictEqual(result.json, 'abc')
assert.strictEqual(getEnforceString(result.json, result.documentState, []), true)
assert.strictEqual((result.documentState as ValueDocumentState).enforceString, true)
})

describe('documentStatePatch', () => {
Expand Down
14 changes: 10 additions & 4 deletions src/lib/logic/documentState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import type {
ArrayDocumentState,
CaretPosition,
DocumentState,
JSONParser,
ObjectDocumentState,
OnExpand,
ArrayRecursiveState,
Expand Down Expand Up @@ -485,6 +484,14 @@ function _documentStatePatch(
}

if (isJSONPatchReplace(operation)) {
const path = parsePath(json, operation.path)
const enforceString = getEnforceString(json, documentState, path)
if (enforceString) {
// ensure the enforceString setting is not lost when for example changing "123"
// into "abc" and later back to "123", so we now make it explicit.
return setInDocumentState(json, documentState, path, { type: 'value', enforceString })
}

// nothing special to do (all is handled by syncDocumentState)
return documentState
}
Expand Down Expand Up @@ -701,8 +708,7 @@ function mergeAdjacentSections(visibleSections: VisibleSection[]): VisibleSectio
export function getEnforceString(
json: unknown,
documentState: DocumentState | undefined,
path: JSONPath,
parser: JSONParser
path: JSONPath
): boolean {
const value = getIn(json, path)
const nestedState = getInRecursiveState(json, documentState, path)
Expand All @@ -712,7 +718,7 @@ export function getEnforceString(
return enforceString
}

return isStringContainingPrimitiveValue(value, parser)
return isStringContainingPrimitiveValue(value)
}

export function getNextKeys(keys: string[], key: string, includeKey = false): string[] {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/logic/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { groupBy, isEmpty, isEqual, mapValues, partition } from 'lodash-es'
import type { JSONSelection, SortedColumn, TableCellIndex, ValidationError } from '$lib/types.js'
import { ValidationSeverity } from '$lib/types.js'
import { createValueSelection, getFocusPath, pathStartsWith } from './selection.js'
import { isNumber } from '../utils/numberUtils.js'
import { containsNumber } from '../utils/numberUtils.js'
import type { Dictionary } from 'lodash'
import { stringifyJSONPath } from '$lib/utils/pathUtils.js'
import { forEachSample } from '$lib/utils/arrayUtils.js'
Expand Down Expand Up @@ -356,7 +356,7 @@ export function groupValidationErrors(
columns: JSONPath[]
): GroupedValidationErrors {
const [arrayErrors, rootErrors] = partition(validationErrors, (validationError) =>
isNumber(validationError.path[0])
containsNumber(validationError.path[0])
)

const errorsByRow: Dictionary<ValidationError[]> = groupBy(arrayErrors, findRowIndex)
Expand Down
2 changes: 1 addition & 1 deletion src/lib/utils/numberUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function isDigit(char: string): boolean {
const DIGIT_REGEX = /^[0-9]$/

// TODO: unit test
export function isNumber(value: string): boolean {
export function containsNumber(value: string): boolean {
return NUMBER_REGEX.test(value)
}

Expand Down
16 changes: 8 additions & 8 deletions src/lib/utils/typeUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ describe('typeUtils', () => {
})

test('isStringContainingPrimitiveValue', () => {
strictEqual(isStringContainingPrimitiveValue(22, JSON), false)
strictEqual(isStringContainingPrimitiveValue('text', JSON), false)
strictEqual(isStringContainingPrimitiveValue('2.4', JSON), true)
strictEqual(isStringContainingPrimitiveValue('-2.4', JSON), true)
strictEqual(isStringContainingPrimitiveValue('2e3', JSON), true)
strictEqual(isStringContainingPrimitiveValue('true', JSON), true)
strictEqual(isStringContainingPrimitiveValue('false', JSON), true)
strictEqual(isStringContainingPrimitiveValue('null', JSON), true)
strictEqual(isStringContainingPrimitiveValue(22), false)
strictEqual(isStringContainingPrimitiveValue('text'), false)
strictEqual(isStringContainingPrimitiveValue('2.4'), true)
strictEqual(isStringContainingPrimitiveValue('-2.4'), true)
strictEqual(isStringContainingPrimitiveValue('2e3'), true)
strictEqual(isStringContainingPrimitiveValue('true'), true)
strictEqual(isStringContainingPrimitiveValue('false'), true)
strictEqual(isStringContainingPrimitiveValue('null'), true)
})

test('isInteger', () => {
Expand Down
12 changes: 7 additions & 5 deletions src/lib/utils/typeUtils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// TODO: unit test typeUtils.js

import { isNumber } from './numberUtils.js'
import { containsNumber } from './numberUtils.js'
import type { JSONParser } from '../types.js'

/**
Expand Down Expand Up @@ -134,7 +134,7 @@ export function valueType(value: unknown, parser: JSONParser): string {

// unknown type (like a LosslessNumber). Try out what stringfying results in
const valueStr = parser.stringify(value)
if (valueStr && isNumber(valueStr)) {
if (valueStr && containsNumber(valueStr)) {
return 'number'
}
if (valueStr === 'true' || valueStr === 'false') {
Expand Down Expand Up @@ -179,7 +179,7 @@ export function stringConvert(str: string, parser: JSONParser): unknown {
return false
}

if (isNumber(strTrim)) {
if (containsNumber(strTrim)) {
return parser.parse(strTrim)
}

Expand All @@ -190,8 +190,10 @@ export function stringConvert(str: string, parser: JSONParser): unknown {
* Test whether a string contains a numeric, boolean, or null value.
* Returns true when the string contains a number, boolean, or null.
*/
export function isStringContainingPrimitiveValue(str: unknown, parser: JSONParser): boolean {
return typeof str === 'string' && typeof stringConvert(str, parser) !== 'string'
export function isStringContainingPrimitiveValue(str: unknown): boolean {
// note that we can safely use JSON parser here instead of the configured JSONParser,
// since we do not actually use the parsed number, just want to check that it is not a string
return typeof str === 'string' && typeof stringConvert(str, JSON) !== 'string'
}

/**
Expand Down

0 comments on commit 2ee127b

Please sign in to comment.