Skip to content

Commit

Permalink
fix: #226 return null instead of { validationErrors: [] } when th…
Browse files Browse the repository at this point in the history
…ere are no validation errors

BREAKING CHANGE: when there are no validation errors, the return value of
method `validate()` and parameter `contentErrors` in callback `onChange` is
now `null` instead of `{ validationErrors: [] }`.
  • Loading branch information
josdejong committed Feb 28, 2023
1 parent 5df57ee commit 395dbd1
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 45 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ const editor = new JSONEditor({

- `onError(err: Error)`.
Callback fired when an error occurs. Default implementation is to log an error in the console and show a simple alert message to the user.
- `onChange(content: Content, previousContent: Content, changeStatus: { contentErrors: ContentErrors, patchResult: JSONPatchResult | null })`. The callback which is invoked on every change of the contents, both changes made by a user and programmatic changes made via methods like `.set()`, `.update()`, or `.patch()`.
- `onChange(content: Content, previousContent: Content, changeStatus: { contentErrors: ContentErrors | null, patchResult: JSONPatchResult | null })`. The callback which is invoked on every change of the contents, both changes made by a user and programmatic changes made via methods like `.set()`, `.update()`, or `.patch()`.
The returned `content` is sometimes of type `{ json }`, and sometimes of type `{ text }`. Which of the two is returned depends on the mode of the editor, the change that is applied, and the state of the document (valid, invalid, empty). Please be aware that `{ text }` can contain invalid JSON: whilst typing in `text` mode, a JSON document will be temporarily invalid, like when the user is typing a new string. The parameter `patchResult` is only returned on changes that can be represented as a JSON Patch document, and for example not when freely typing in `text` mode.
- `onChangeMode(mode: 'tree' | 'text' | 'table')`. Invoked when the mode is changed.
- `onClassName(path: Path, value: any): string | undefined`.
Expand Down Expand Up @@ -351,7 +351,7 @@ const editor = new JSONEditor({
- `findElement(path: Path)` Find the DOM element of a given path. Returns `null` when not found.
- `acceptAutoRepair(): Content` In tree mode, invalid JSON is automatically repaired when loaded. When the repair was successful, the repaired contents are rendered but not yet applied to the document itself until the user clicks "Ok" or starts editing the data. Instead of accepting the repair, the user can also click "Repair manually instead". Invoking `.acceptAutoRepair()` will programmatically accept the repair. This will trigger an update, and the method itself also returns the updated contents. In case of `text` mode or when the editor is not in an "accept auto repair" status, nothing will happen, and the contents will be returned as is.
- `refresh()`. Refresh rendering of the contents, for example after changing the font size. This is only available in `text` mode.
- `validate() : ContentErrors`. Get all current parse errors and validation errors.
- `validate() : ContentErrors | null`. Get all current parse errors and validation errors.
- `focus()`. Give the editor focus.
- `destroy()`. Destroy the editor, remove it from the DOM.

Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/JSONEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@
* Validate the contents of the editor using the configured validator.
* Returns a parse error or a list with validation warnings
*/
export function validate(): ContentErrors {
export function validate(): ContentErrors | null {
return refJSONEditorRoot.validate()
}
Expand Down
2 changes: 1 addition & 1 deletion src/lib/components/modes/JSONEditorRoot.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
* Validate the contents of the editor using the configured validator.
* Returns a parse error or a list with validation warnings
*/
export function validate(): ContentErrors {
export function validate(): ContentErrors | null {
if (refTextMode) {
return refTextMode.validate()
} else if (refTreeMode) {
Expand Down
6 changes: 2 additions & 4 deletions src/lib/components/modes/tablemode/TableMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@
)
}
export function validate(): ContentErrors {
export function validate(): ContentErrors | null {
debug('validate')
if (parseError) {
Expand All @@ -540,9 +540,7 @@
// make sure the validation results are up-to-date
// normally, they are only updated on the next tick after the json is changed
updateValidationErrors(json, validator, parser, validationParser)
return {
validationErrors
}
return !isEmpty(validationErrors) ? { validationErrors } : null
}
export function patch(
Expand Down
4 changes: 2 additions & 2 deletions src/lib/components/modes/textmode/TextMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,7 @@
return []
}
export function validate(): ContentErrors {
export function validate(): ContentErrors | null {
debug('validate:start')
onChangeCodeMirrorValueDebounced.flush()
Expand All @@ -799,7 +799,7 @@
} else {
jsonStatus = JSON_STATUS_VALID
jsonParseError = null
validationErrors = contentErrors.validationErrors
validationErrors = contentErrors?.validationErrors || []
}
debug('validate:end')
Expand Down
6 changes: 2 additions & 4 deletions src/lib/components/modes/treemode/TreeMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@
)
}
export function validate(): ContentErrors {
export function validate(): ContentErrors | null {
debug('validate')
if (parseError) {
Expand All @@ -449,9 +449,7 @@
// make sure the validation results are up-to-date
// normally, they are only updated on the next tick after the json is changed
updateValidationErrors(json, validator, parser, validationParser)
return {
validationErrors
}
return !isEmpty(validationErrors) ? { validationErrors } : null
}
export function getJson() {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/logic/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ export function onRemove({
{ text: '', json: undefined },
json !== undefined ? { text, json } : { text: text || '', json },
{
contentErrors: { validationErrors: [] },
contentErrors: null,
patchResult: null
}
)
Expand Down
12 changes: 3 additions & 9 deletions src/lib/logic/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,7 @@ describe('validation', () => {
const invalidText = '{ "foo": 42 }'

test('should validateText with native parser and valid JSON', () => {
deepStrictEqual(validateText(validText, myValidator, JSON, JSON), {
validationErrors: []
})
deepStrictEqual(validateText(validText, myValidator, JSON, JSON), null)
})

test('should validateText with native parser and invalid JSON', () => {
Expand All @@ -176,9 +174,7 @@ describe('validation', () => {
})

test('should validateText with lossless parser and valid JSON', () => {
deepStrictEqual(validateText(validText, myValidator, LosslessJSONParser, JSON), {
validationErrors: []
})
deepStrictEqual(validateText(validText, myValidator, LosslessJSONParser, JSON), null)
})

test('should validateText with lossless parser and invalid JSON', () => {
Expand All @@ -190,9 +186,7 @@ describe('validation', () => {
test('should validateText with two lossless parsers and valid JSON', () => {
deepStrictEqual(
validateText(validText, myLosslessValidator, LosslessJSONParser, LosslessJSONParser),
{
validationErrors: []
}
null
)
})

Expand Down
14 changes: 5 additions & 9 deletions src/lib/logic/validation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { initial } from 'lodash-es'
import { initial, isEmpty } from 'lodash-es'
import type {
ContentErrors,
JSONParser,
Expand Down Expand Up @@ -82,7 +82,7 @@ export function validateText(
validator: Validator | null,
parser: JSONParser,
validationParser: JSONParser
): ContentErrors {
): ContentErrors | null {
debug('validateText')

if (text.length > MAX_VALIDATABLE_SIZE) {
Expand All @@ -99,9 +99,7 @@ export function validateText(

if (text.length === 0) {
// new, empty document, do not try to parse
return {
validationErrors: []
}
return null
}

try {
Expand All @@ -113,9 +111,7 @@ export function validateText(
)

if (!validator) {
return {
validationErrors: []
}
return null
}

// if needed, parse with the validationParser to be able to feed the json to the validator
Expand All @@ -133,7 +129,7 @@ export function validateText(
(duration) => debug(`validate: validated json in ${duration} ms`)
)

return { validationErrors }
return !isEmpty(validationErrors) ? { validationErrors } : null
} catch (err) {
const isRepairable = measure(
() => canAutoRepair(text, parser),
Expand Down
16 changes: 5 additions & 11 deletions src/lib/typeguards.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type {
ContentErrors,
ContentParseError,
ContentValidationErrors,
ContextMenuColumn,
Expand All @@ -11,6 +10,7 @@ import type {
MenuSpace,
MenuSpaceItem
} from './types.js'
import { isObject } from '$lib/utils/typeUtils'

export function isMenuSpaceItem(item: unknown): item is MenuSpaceItem {
return isMenuSpace(item)
Expand Down Expand Up @@ -69,18 +69,12 @@ export function isContextMenuColumn(item: unknown): item is ContextMenuColumn {
return item ? item['type'] === 'column' && Array.isArray(item['items']) : false
}

export function isContentParseError(
contentErrors: ContentErrors
): contentErrors is ContentParseError {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
return typeof contentErrors['parseError'] === 'object' && contentErrors['parseError'] !== null
export function isContentParseError(contentErrors: unknown): contentErrors is ContentParseError {
return isObject(contentErrors) && isObject(contentErrors['parseError'])
}

export function isContentValidationErrors(
contentErrors: ContentErrors
contentErrors: unknown
): contentErrors is ContentValidationErrors {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
return Array.isArray(contentErrors['validationErrors'])
return isObject(contentErrors) && Array.isArray(contentErrors['validationErrors'])
}
2 changes: 1 addition & 1 deletion src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ export interface QueryLanguageOptions {

export type OnChangeQueryLanguage = (queryLanguageId: string) => void
export interface OnChangeStatus {
contentErrors: ContentErrors
contentErrors: ContentErrors | null
patchResult: JSONPatchResult | null
}
export type OnChange =
Expand Down

0 comments on commit 395dbd1

Please sign in to comment.