From 6062a9a34b61fa0ea92ba217e0428fc1d2ccc519 Mon Sep 17 00:00:00 2001 From: Per-Kristian Nordnes Date: Wed, 22 May 2024 12:43:48 +0200 Subject: [PATCH] refactor(portable-text-editor): automatically resolve some validation errors (#6705) * fix(portable-text-editor): normalize missing .markDefs on text block schema type * refactor(portable-text-editor): refactor addAnnotation fn This was too damn messy and contained a bug where sometimes an annotation would not be added. Fixed the bug. * refactor(portable-text-editor): don't automatically add missing markDefs to editor value We will normalize this automatically and update the value when needed. For this to work, the value must reflect the original value * feat(portable-text-editor): normalize away marks that doesn't exist in the the schema Note: you need to actively edit the node before it's normalized * test(portable-text-editor): update tests to test for new normalization of marks and markdefs * chore(portable-text-editor): use tsx over ts-node * refactor(portable-text-editor): automatically resolve some validation error Resolve errors for missing .markDefs, orphapned marks, empty children, missing child keys * fix(portable-text-editor): allow for missing span marks in value Don't insert a empty array for missing span node .marks. It will be inserted through normalization inside withPortableTextMarkModel instead. * doc(portable-text-editor): update comments * fix(portable-text-editor): validate missing and empty children * test(portable-text-editor): update test to test for missing children * fix(types): fix invalid assertion in isPortableTextTextBlock As .markDefs is optional, return true when missing * fix(portable-text-editor): don't validate or normalize missing .markDefs It should be allowed and is a optional property according to the schema --- .../@sanity/portable-text-editor/package.json | 3 +- .../__tests__/PortableTextEditor.test.tsx | 53 ++--- .../__tests__/pteWarningsSelfSolving.test.tsx | 182 ++++++++++++++++++ .../src/editor/hooks/useSyncValue.ts | 22 ++- .../withPortableTextMarkModel.test.tsx | 36 ++-- .../editor/plugins/createWithEditableAPI.ts | 117 ++++++----- .../createWithPortableTextMarkModel.ts | 28 +++ .../editor/plugins/createWithSchemaTypes.ts | 5 +- .../portable-text-editor/src/types/editor.ts | 1 + .../__tests__/pteWarningsSelfSolving.test.tsx | 58 ------ .../src/utils/__tests__/values.test.ts | 4 - .../src/utils/validateValue.ts | 115 ++++++----- .../portable-text-editor/src/utils/values.ts | 64 +++--- .../types/src/portableText/asserters.ts | 2 +- pnpm-lock.yaml | 20 ++ 15 files changed, 454 insertions(+), 256 deletions(-) create mode 100644 packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx delete mode 100644 packages/@sanity/portable-text-editor/src/utils/__tests__/pteWarningsSelfSolving.test.tsx diff --git a/packages/@sanity/portable-text-editor/package.json b/packages/@sanity/portable-text-editor/package.json index 358c2679887..3653c4d2979 100644 --- a/packages/@sanity/portable-text-editor/package.json +++ b/packages/@sanity/portable-text-editor/package.json @@ -48,7 +48,7 @@ "build": "pkg-utils build --strict --check --clean", "check:types": "tsc --project tsconfig.lib.json", "clean": "rimraf lib", - "dev": "cd ./e2e-tests/ && ts-node serve", + "dev": "cd ./e2e-tests/ && tsx serve", "lint": "eslint .", "prepublishOnly": "turbo run build", "prettier": "prettier --write './**/*.{ts,tsx,js,css,html}'", @@ -97,6 +97,7 @@ "rimraf": "^3.0.2", "rxjs": "^7.8.1", "styled-components": "^6.1.11", + "tsx": "^4.10.3", "vite": "^4.5.3" }, "peerDependencies": { diff --git a/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx b/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx index b2f0efdee8e..b40dd18cf8a 100644 --- a/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx +++ b/packages/@sanity/portable-text-editor/src/editor/__tests__/PortableTextEditor.test.tsx @@ -330,19 +330,12 @@ describe('initialization', () => { await waitFor(() => { if (editorRef.current) { expect(onChange).toHaveBeenCalledWith({ + type: 'invalidValue', + value: initialValue, resolution: { action: 'Remove invalid marks', description: "Block with _key 'abc' contains marks (invalid) not supported by the current content model.", - item: { - _key: 'abc', - _type: 'myTestBlockType', - children: [{_key: 'def', _type: 'span', marks: ['invalid'], text: 'Test'}], - markDefs: [], - }, - patches: [ - {path: [{_key: 'abc'}, 'children', {_key: 'def'}, 'marks'], type: 'set', value: []}, - ], i18n: { action: 'inputs.portable-text.invalid-value.orphaned-marks.action', description: 'inputs.portable-text.invalid-value.orphaned-marks.description', @@ -351,26 +344,40 @@ describe('initialization', () => { orphanedMarks: ['invalid'], }, }, - }, - type: 'invalidValue', - value: [ - { - _key: '123', - _type: 'myTestBlockType', - children: [{_key: '567', _type: 'span', marks: [], text: 'Hello'}], - markDefs: [], - }, - { + item: { _key: 'abc', _type: 'myTestBlockType', - children: [{_key: 'def', _type: 'span', marks: ['invalid'], text: 'Test'}], + children: [ + { + _key: 'def', + _type: 'span', + marks: ['invalid'], + text: 'Test', + }, + ], markDefs: [], }, - ], + patches: [ + { + path: [ + { + _key: 'abc', + }, + 'children', + { + _key: 'def', + }, + 'marks', + ], + type: 'set', + value: [], + }, + ], + }, }) - expect(onChange).not.toHaveBeenCalledWith({type: 'value', value: initialValue}) - expect(onChange).toHaveBeenCalledWith({type: 'ready'}) } }) + expect(onChange).not.toHaveBeenCalledWith({type: 'value', value: initialValue}) + expect(onChange).toHaveBeenCalledWith({type: 'ready'}) }) }) diff --git a/packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx b/packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx new file mode 100644 index 00000000000..c08f4878402 --- /dev/null +++ b/packages/@sanity/portable-text-editor/src/editor/__tests__/pteWarningsSelfSolving.test.tsx @@ -0,0 +1,182 @@ +import {describe, expect, it, jest} from '@jest/globals' +import {render, waitFor} from '@testing-library/react' +import {createRef, type RefObject} from 'react' + +import {PortableTextEditor} from '../PortableTextEditor' +import {PortableTextEditorTester, schemaType} from './PortableTextEditorTester' + +describe('when PTE would display warnings, instead it self solves', () => { + it('when child at index is missing required _key in block with _key', async () => { + const editorRef: RefObject = createRef() + const initialValue = [ + { + _key: 'abc', + _type: 'myTestBlockType', + children: [ + { + _type: 'span', + marks: [], + text: 'Hello with a new key', + }, + ], + markDefs: [], + style: 'normal', + }, + ] + + const onChange = jest.fn() + render( + , + ) + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue}) + expect(onChange).toHaveBeenCalledWith({type: 'ready'}) + }) + await waitFor(() => { + if (editorRef.current) { + expect(PortableTextEditor.getValue(editorRef.current)).toEqual([ + { + _key: 'abc', + _type: 'myTestBlockType', + children: [ + { + _key: '4', + _type: 'span', + text: 'Hello with a new key', + marks: [], + }, + ], + markDefs: [], + style: 'normal', + }, + ]) + } + }) + }) + + it('allows missing .markDefs', async () => { + const editorRef: RefObject = createRef() + const initialValue = [ + { + _key: 'abc', + _type: 'myTestBlockType', + children: [ + { + _key: 'def', + _type: 'span', + marks: [], + text: 'No markDefs', + }, + ], + style: 'normal', + }, + ] + + const onChange = jest.fn() + render( + , + ) + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue}) + expect(onChange).toHaveBeenCalledWith({type: 'ready'}) + }) + await waitFor(() => { + if (editorRef.current) { + PortableTextEditor.focus(editorRef.current) + expect(PortableTextEditor.getValue(editorRef.current)).toEqual([ + { + _key: 'abc', + _type: 'myTestBlockType', + children: [ + { + _key: 'def', + _type: 'span', + text: 'No markDefs', + marks: [], + }, + ], + style: 'normal', + }, + ]) + } + }) + }) + + it('adds missing .children', async () => { + const editorRef: RefObject = createRef() + const initialValue = [ + { + _key: 'abc', + _type: 'myTestBlockType', + style: 'normal', + markDefs: [], + }, + { + _key: 'def', + _type: 'myTestBlockType', + style: 'normal', + children: [], + markDefs: [], + }, + ] + + const onChange = jest.fn() + render( + , + ) + await waitFor(() => { + expect(onChange).toHaveBeenCalledWith({type: 'value', value: initialValue}) + expect(onChange).toHaveBeenCalledWith({type: 'ready'}) + }) + await waitFor(() => { + if (editorRef.current) { + PortableTextEditor.focus(editorRef.current) + expect(PortableTextEditor.getValue(editorRef.current)).toEqual([ + { + _key: 'abc', + _type: 'myTestBlockType', + children: [ + { + _key: '5', + _type: 'span', + text: '', + marks: [], + }, + ], + markDefs: [], + style: 'normal', + }, + { + _key: 'def', + _type: 'myTestBlockType', + children: [ + { + _key: '6', + _type: 'span', + text: '', + marks: [], + }, + ], + markDefs: [], + style: 'normal', + }, + ]) + } + }) + }) +}) diff --git a/packages/@sanity/portable-text-editor/src/editor/hooks/useSyncValue.ts b/packages/@sanity/portable-text-editor/src/editor/hooks/useSyncValue.ts index c490305f0b8..ed0d2ddd7f3 100644 --- a/packages/@sanity/portable-text-editor/src/editor/hooks/useSyncValue.ts +++ b/packages/@sanity/portable-text-editor/src/editor/hooks/useSyncValue.ts @@ -142,7 +142,24 @@ export function useSyncValue( if (hasChanges && isValid) { const validationValue = [value[currentBlockIndex]] const validation = validateValue(validationValue, schemaTypes, keyGenerator) - if (validation.valid) { + // Resolve validations that can be resolved automatically, without involving the user (but only if the value was changed) + if ( + !validation.valid && + validation.resolution?.autoResolve && + validation.resolution?.patches.length > 0 + ) { + // Only apply auto resolution if the value has been populated before and is different from the last one. + if (!readOnly && previousValue.current && previousValue.current !== value) { + // Give a console warning about the fact that it did an auto resolution + console.warn( + `${validation.resolution.action} for block with _key '${validationValue[0]._key}'. ${validation.resolution?.description}`, + ) + validation.resolution.patches.forEach((patch) => { + change$.next({type: 'patch', patch}) + }) + } + } + if (validation.valid || validation.resolution?.autoResolve) { if (oldBlock._key === currentBlock._key) { if (debug.enabled) debug('Updating block', oldBlock, currentBlock) _updateBlock(slateEditor, currentBlock, oldBlock, currentBlockIndex) @@ -168,7 +185,7 @@ export function useSyncValue( 'Validating and inserting new block in the end of the value', currentBlock, ) - if (validation.valid) { + if (validation.valid || validation.resolution?.autoResolve) { withPreserveKeys(slateEditor, () => { Transforms.insertNodes(slateEditor, currentBlock, { at: [currentBlockIndex], @@ -255,6 +272,7 @@ function _replaceBlock( withPreserveKeys(slateEditor, () => { Transforms.insertNodes(slateEditor, currentBlock, {at: [currentBlockIndex]}) }) + slateEditor.onChange() if (selectionFocusOnBlock) { Transforms.select(slateEditor, currentSelection) } diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/__tests__/withPortableTextMarkModel.test.tsx b/packages/@sanity/portable-text-editor/src/editor/plugins/__tests__/withPortableTextMarkModel.test.tsx index e613d12b94a..c707fdcf2d6 100644 --- a/packages/@sanity/portable-text-editor/src/editor/plugins/__tests__/withPortableTextMarkModel.test.tsx +++ b/packages/@sanity/portable-text-editor/src/editor/plugins/__tests__/withPortableTextMarkModel.test.tsx @@ -203,7 +203,7 @@ describe('plugin:withPortableTextMarksModel', () => { focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0}, anchor: {path: [{_key: 'b'}, 'children', {_key: 'b1'}], offset: 1}, }) - PortableTextEditor.toggleMark(editor, 'bold') + PortableTextEditor.toggleMark(editor, 'strong') const value = PortableTextEditor.getValue(editor) expect(value).toMatchInlineSnapshot(` Array [ @@ -215,7 +215,7 @@ describe('plugin:withPortableTextMarksModel', () => { "_key": "a1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "123", }, @@ -231,7 +231,7 @@ describe('plugin:withPortableTextMarksModel', () => { "_key": "b1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "1", }, @@ -287,7 +287,7 @@ describe('plugin:withPortableTextMarksModel', () => { focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0}, anchor: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 4}, }) - PortableTextEditor.toggleMark(editor, 'bold') + PortableTextEditor.toggleMark(editor, 'strong') expect(PortableTextEditor.getValue(editor)).toMatchInlineSnapshot(` Array [ Object { @@ -298,7 +298,7 @@ describe('plugin:withPortableTextMarksModel', () => { "_key": "a1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "1234", }, @@ -315,7 +315,7 @@ describe('plugin:withPortableTextMarksModel', () => { focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 1}, anchor: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 3}, }) - PortableTextEditor.toggleMark(editorRef.current, 'bold') + PortableTextEditor.toggleMark(editorRef.current, 'strong') expect(PortableTextEditor.getValue(editorRef.current)).toMatchInlineSnapshot(` Array [ Object { @@ -326,7 +326,7 @@ describe('plugin:withPortableTextMarksModel', () => { "_key": "a1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "1", }, @@ -340,7 +340,7 @@ describe('plugin:withPortableTextMarksModel', () => { "_key": "1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "4", }, @@ -358,7 +358,7 @@ describe('plugin:withPortableTextMarksModel', () => { focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0}, anchor: {path: [{_key: 'a'}, 'children', {_key: '1'}], offset: 1}, }) - PortableTextEditor.toggleMark(editor, 'bold') + PortableTextEditor.toggleMark(editor, 'strong') expect(PortableTextEditor.getValue(editor)).toMatchInlineSnapshot(` Array [ Object { @@ -369,7 +369,7 @@ Array [ "_key": "a1", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": "1234", }, @@ -432,7 +432,7 @@ Array [ focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0}, anchor: {path: [{_key: 'a'}, 'children', {_key: 'b1'}], offset: 12}, }) - PortableTextEditor.toggleMark(editor, 'bold') + PortableTextEditor.toggleMark(editor, 'strong') expect(PortableTextEditor.getValue(editor)).toMatchInlineSnapshot(` Array [ Object { @@ -444,7 +444,7 @@ Array [ "_type": "span", "marks": Array [ "abc", - "bold", + "strong", ], "text": "A link", }, @@ -452,7 +452,7 @@ Array [ "_key": "a2", "_type": "span", "marks": Array [ - "bold", + "strong", ], "text": ", not a link", }, @@ -778,7 +778,7 @@ Array [ { _key: 'a1', _type: 'span', - marks: ['bold'], + marks: ['strong'], text: '12', }, { @@ -788,7 +788,7 @@ Array [ text: '34', }, ], - markDefs: [{_key: 'bold', _type: 'strong'}], + markDefs: [{_key: 'strong', _type: 'strong'}], style: 'normal', }, ] @@ -811,9 +811,9 @@ Array [ focus: {path: [{_key: 'a'}, 'children', {_key: 'a1'}], offset: 0}, anchor: {path: [{_key: 'a'}, 'children', {_key: '2'}], offset: 2}, }) - expect(PortableTextEditor.isMarkActive(editor, 'bold')).toBe(false) - PortableTextEditor.toggleMark(editor, 'bold') - expect(PortableTextEditor.isMarkActive(editor, 'bold')).toBe(true) + expect(PortableTextEditor.isMarkActive(editor, 'strong')).toBe(false) + PortableTextEditor.toggleMark(editor, 'strong') + expect(PortableTextEditor.isMarkActive(editor, 'strong')).toBe(true) }) }) diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts index 3a1d61d606c..14ff10f265a 100644 --- a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts +++ b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithEditableAPI.ts @@ -341,12 +341,24 @@ export function createWithEditableAPI( type: ObjectSchemaType, value?: {[prop: string]: unknown}, ): {spanPath: Path; markDefPath: Path} | undefined => { - const {selection} = editor - if (selection) { - const [block] = Editor.node(editor, selection.focus, {depth: 1}) - if (SlateElement.isElement(block) && block._type === types.block.name) { - const annotationKey = keyGenerator() - if (editor.isTextBlock(block)) { + const {selection: originalSelection} = editor + let returnValue: {spanPath: Path; markDefPath: Path} | undefined = undefined + if (originalSelection) { + const [block] = Editor.node(editor, originalSelection.focus, {depth: 1}) + if (!editor.isTextBlock(block)) { + return undefined + } + if (Range.isCollapsed(originalSelection)) { + editor.pteExpandToWord() + editor.onChange() + } + const [textNode] = Editor.node(editor, originalSelection.focus, {depth: 2}) + + // If we still have a selection, add the annotation to the selected text + if (editor.selection) { + Editor.withoutNormalizing(editor, () => { + // Add markDefs to the block + const annotationKey = keyGenerator() Transforms.setNodes( editor, { @@ -355,68 +367,55 @@ export function createWithEditableAPI( {_type: type.name, _key: annotationKey, ...value} as PortableTextObject, ], }, - {at: selection.focus}, + {at: originalSelection.focus}, ) editor.onChange() - if (Range.isCollapsed(selection)) { - editor.pteExpandToWord() - editor.onChange() + + // Split if needed + Transforms.setNodes(editor, {}, {match: Text.isText, split: true}) + editor.onChange() + + // Add marks to the span node + if (editor.selection && Text.isText(textNode)) { + Transforms.setNodes( + editor, + { + marks: [...((textNode.marks || []) as string[]), annotationKey], + }, + { + at: editor.selection, + match: (n) => n._type === types.span.name, + }, + ) } - const [textNode] = Editor.node(editor, selection.focus, {depth: 2}) + editor.onChange() if (editor.selection) { - Editor.withoutNormalizing(editor, () => { - // Split if needed - Transforms.setNodes(editor, {}, {match: Text.isText, split: true}) - if (editor.selection && Text.isText(textNode)) { - Transforms.setNodes( - editor, - { - marks: [...((textNode.marks || []) as string[]), annotationKey], - }, - { - at: editor.selection, - match: (n) => n._type === types.span.name, - }, - ) - editor.onChange() - } - }) - Editor.normalize(editor) - editor.onChange() - const newSelection = toPortableTextRange( - fromSlateValue( - editor.children, - types.block.name, - KEY_TO_VALUE_ELEMENT.get(editor), - ), - editor.selection, - types, + // Insert an empty string to continue writing non-annotated text + Transforms.insertNodes( + editor, + [{_type: 'span', text: '', marks: [], _key: keyGenerator()}], + { + at: Range.end(editor.selection), + }, ) - // eslint-disable-next-line max-depth - if (newSelection && typeof block._key === 'string') { - // Insert an empty string to continue writing non-annotated text - Editor.withoutNormalizing(editor, () => { - if (editor.selection) { - Transforms.insertNodes( - editor, - [{_type: 'span', text: '', marks: [], _key: keyGenerator()}], - { - at: Range.end(editor.selection), - }, - ) - editor.onChange() - } - }) - return { - spanPath: newSelection.focus.path, - markDefPath: [{_key: block._key}, 'markDefs', {_key: annotationKey}], - } + } + const newPortableTextEditorSelection = toPortableTextRange( + fromSlateValue(editor.children, types.block.name, KEY_TO_VALUE_ELEMENT.get(editor)), + editor.selection, + types, + ) + if (newPortableTextEditorSelection) { + returnValue = { + spanPath: newPortableTextEditorSelection.focus.path, + markDefPath: [{_key: block._key}, 'markDefs', {_key: annotationKey}], } } - } + }) + Editor.normalize(editor) + editor.onChange() } } - return undefined + return returnValue }, delete: (selection: EditorSelection, options?: EditableAPIDeleteOptions): void => { if (selection) { diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts index 36d187d6a6e..242e5157603 100644 --- a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts +++ b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithPortableTextMarkModel.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-statements */ /* eslint-disable complexity */ /** * @@ -16,6 +17,7 @@ import { } from '../../types/editor' import {debugWithName} from '../../utils/debug' import {toPortableTextRange} from '../../utils/ranges' +import {EMPTY_MARKS} from '../../utils/values' const debug = debugWithName('plugin:withPortableTextMarkModel') @@ -72,6 +74,32 @@ export function createWithPortableTextMarkModel( Transforms.setNodes(editor, {marks: []}, {at: path}) editor.onChange() } + const hasSpanMarks = isSpan && (node.marks || []).length > 0 + if (hasSpanMarks) { + const spanMarks = node.marks || EMPTY_MARKS + // Test that every annotation mark used has a definition in markDefs + const annotationMarks = spanMarks.filter( + (mark) => !types.decorators.map((dec) => dec.value).includes(mark), + ) + if (annotationMarks.length > 0) { + const [block] = Editor.node(editor, Path.parent(path)) + const orphanedMarks = + (editor.isTextBlock(block) && + annotationMarks.filter( + (mark) => !block.markDefs?.find((def) => def._key === mark), + )) || + [] + if (orphanedMarks.length > 0) { + debug('Removing orphaned .marks from span node') + Transforms.setNodes( + editor, + {marks: spanMarks.filter((mark) => !orphanedMarks.includes(mark))}, + {at: path}, + ) + editor.onChange() + } + } + } for (const op of editor.operations) { // Make sure markDefs are copied over when merging two blocks. if ( diff --git a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithSchemaTypes.ts b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithSchemaTypes.ts index 6ba4afcaf3d..8b35b1e2c36 100644 --- a/packages/@sanity/portable-text-editor/src/editor/plugins/createWithSchemaTypes.ts +++ b/packages/@sanity/portable-text-editor/src/editor/plugins/createWithSchemaTypes.ts @@ -49,10 +49,12 @@ export function createWithSchemaTypes({ ) } - // Extend Slate's default normalization to add `_type: 'span'` to texts if they are inserted without + // Extend Slate's default normalization const {normalizeNode} = editor editor.normalizeNode = (entry) => { const [node, path] = entry + + // If text block children node is missing _type, set it to the span type if (node._type === undefined && path.length === 2) { debug('Setting span type on text node without a type') const span = node as PortableTextSpan @@ -66,6 +68,7 @@ export function createWithSchemaTypes({ const key = keyGenerator() Transforms.setNodes(editor, {_key: key}, {at: path}) } + normalizeNode(entry) } return editor diff --git a/packages/@sanity/portable-text-editor/src/types/editor.ts b/packages/@sanity/portable-text-editor/src/types/editor.ts index ae443808ccd..5faccf67ee5 100644 --- a/packages/@sanity/portable-text-editor/src/types/editor.ts +++ b/packages/@sanity/portable-text-editor/src/types/editor.ts @@ -300,6 +300,7 @@ export type ErrorChange = { * The editor has invalid data in the value that can be resolved by the user * @beta */ export type InvalidValueResolution = { + autoResolve?: boolean patches: Patch[] description: string action: string diff --git a/packages/@sanity/portable-text-editor/src/utils/__tests__/pteWarningsSelfSolving.test.tsx b/packages/@sanity/portable-text-editor/src/utils/__tests__/pteWarningsSelfSolving.test.tsx deleted file mode 100644 index e08be784884..00000000000 --- a/packages/@sanity/portable-text-editor/src/utils/__tests__/pteWarningsSelfSolving.test.tsx +++ /dev/null @@ -1,58 +0,0 @@ -import {describe, expect, it, jest} from '@jest/globals' -import {render, waitFor} from '@testing-library/react' -import {createRef, type RefObject} from 'react' - -import {PortableTextEditorTester, schemaType} from '../../editor/__tests__/PortableTextEditorTester' -import {PortableTextEditor} from '../../editor/PortableTextEditor' - -describe('when PTE would display warnings, instead it self solves', () => { - it('when child at index is missing required _key in block with _key', async () => { - const editorRef: RefObject = createRef() - const initialValue = [ - { - _key: '1', - _type: 'myTestBlockType', - children: [ - { - _type: 'span', - marks: [], - text: 'Hello with a new key', - }, - ], - markDefs: [], - style: 'normal', - }, - ] - - const onChange = jest.fn() - render( - , - ) - await waitFor(() => { - if (editorRef.current) { - PortableTextEditor.focus(editorRef.current) - expect(PortableTextEditor.getValue(editorRef.current)).toEqual([ - { - _key: '1', - _type: 'myTestBlockType', - children: [ - { - _key: '3', - _type: 'span', - text: 'Hello with a new key', - marks: [], - }, - ], - markDefs: [], - style: 'normal', - }, - ]) - } - }) - }) -}) diff --git a/packages/@sanity/portable-text-editor/src/utils/__tests__/values.test.ts b/packages/@sanity/portable-text-editor/src/utils/__tests__/values.test.ts index f88639f37a9..c0dffb4f95e 100644 --- a/packages/@sanity/portable-text-editor/src/utils/__tests__/values.test.ts +++ b/packages/@sanity/portable-text-editor/src/utils/__tests__/values.test.ts @@ -68,11 +68,9 @@ Array [ Object { "_key": "1231", "_type": "span", - "marks": Array [], "text": "123", }, ], - "markDefs": Array [], "style": "normal", }, ] @@ -112,7 +110,6 @@ Array [ Object { "_key": "1231", "_type": "span", - "marks": Array [], "text": "123", }, Object { @@ -134,7 +131,6 @@ Array [ }, }, ], - "markDefs": Array [], "style": "normal", }, ] diff --git a/packages/@sanity/portable-text-editor/src/utils/validateValue.ts b/packages/@sanity/portable-text-editor/src/utils/validateValue.ts index 629f1be48cb..a6785506f7c 100644 --- a/packages/@sanity/portable-text-editor/src/utils/validateValue.ts +++ b/packages/@sanity/portable-text-editor/src/utils/validateValue.ts @@ -6,8 +6,9 @@ import { } from '@sanity/types' import {flatten, isPlainObject, uniq} from 'lodash' -import {insert, set, unset} from '../patch/PatchEvent' +import {insert, set, setIfMissing, unset} from '../patch/PatchEvent' import {type InvalidValueResolution, type PortableTextMemberSchemaTypes} from '../types/editor' +import {EMPTY_MARKDEFS} from './values' export interface Validation { valid: boolean @@ -148,15 +149,16 @@ export function validateValue( } return true } - // Test that every child in text block is valid + + // Test regular text blocks if (blk._type === types.block.name) { const textBlock = blk as PortableTextTextBlock - // Test that it has (valid) children property - if (!textBlock.children || !Array.isArray(textBlock.children)) { + // Test that it has a valid children property (array) + if (textBlock.children && !Array.isArray(textBlock.children)) { resolution = { - patches: [unset([{_key: textBlock._key}])], - description: `Text block with _key '${textBlock._key}' has a missing or invalid required property 'children'.`, - action: 'Remove the block', + patches: [set({children: []}, [{_key: textBlock._key}])], + description: `Text block with _key '${textBlock._key}' has a invalid required property 'children'.`, + action: 'Reset the children property', item: textBlock, i18n: { @@ -168,11 +170,40 @@ export function validateValue( } return true } - // Test that markDefs exists and is valid - if (!blk.markDefs || !Array.isArray(blk.markDefs)) { + // Test that children is set and lengthy + if ( + textBlock.children === undefined || + (Array.isArray(textBlock.children) && textBlock.children.length === 0) + ) { + const newSpan = { + _type: types.span.name, + _key: keyGenerator(), + text: '', + marks: [], + } resolution = { - patches: [set({...textBlock, markDefs: []}, [{_key: textBlock._key}])], - description: `Block has a missing or invalid required property 'markDefs'.`, + autoResolve: true, + patches: [ + setIfMissing([], [{_key: blk._key}, 'children']), + insert([newSpan], 'after', [{_key: blk._key}, 'children', 0]), + ], + description: `Children for text block with _key '${blk._key}' is empty.`, + action: 'Insert an empty text', + item: blk, + + i18n: { + description: 'inputs.portable-text.invalid-value.empty-children.description', + action: 'inputs.portable-text.invalid-value.empty-children.action', + values: {key: blk._key}, + }, + } + return true + } + // Test that markDefs are valid if they exists + if (blk.markDefs && !Array.isArray(blk.markDefs)) { + resolution = { + patches: [set({...textBlock, markDefs: EMPTY_MARKDEFS}, [{_key: textBlock._key}])], + description: `Block has invalid required property 'markDefs'.`, action: 'Add empty markDefs array', item: textBlock, @@ -185,23 +216,6 @@ export function validateValue( } return true } - // NOTE: this is commented out as we want to allow the saved data to have optional .marks for spans (as specified by the schema) - // const spansWithUndefinedMarks = blk.children - // .filter(cld => cld._type === types.span.name) - // .filter(cld => typeof cld.marks === 'undefined') - - // if (spansWithUndefinedMarks.length > 0) { - // const first = spansWithUndefinedMarks[0] - // resolution = { - // patches: [ - // set({...first, marks: []}, [{_key: blk._key}, 'children', {_key: first._key}]) - // ], - // description: `Span has no .marks array`, - // action: 'Add empty marks array', - // item: first - // } - // return true - // } const allUsedMarks = uniq( flatten( textBlock.children @@ -234,8 +248,10 @@ export function validateValue( const annotationMarks = allUsedMarks.filter( (mark) => !types.decorators.map((dec) => dec.value).includes(mark), ) - const orphanedMarks = annotationMarks.filter((mark) => - textBlock.markDefs ? !textBlock.markDefs.find((def) => def._key === mark) : false, + const orphanedMarks = annotationMarks.filter( + (mark) => + textBlock.markDefs === undefined || + !textBlock.markDefs.find((def) => def._key === mark), ) if (orphanedMarks.length > 0) { const spanChildren = textBlock.children.filter( @@ -267,27 +283,6 @@ export function validateValue( } } - // Test that children is lengthy - if (textBlock.children && textBlock.children.length === 0) { - const newSpan = { - _type: types.span.name, - _key: keyGenerator(), - text: '', - } - resolution = { - patches: [insert([newSpan], 'after', [{_key: blk._key}, 'children', 0])], - description: `Children for text block with _key '${blk._key}' is empty.`, - action: 'Insert an empty text', - item: blk, - - i18n: { - description: 'inputs.portable-text.invalid-value.empty-children.description', - action: 'inputs.portable-text.invalid-value.empty-children.action', - values: {key: blk._key}, - }, - } - return true - } // Test every child if ( textBlock.children.some((child, cIndex: number) => { @@ -307,6 +302,24 @@ export function validateValue( return true } + if (!child._key || typeof child._key !== 'string') { + const newChild = {...child, _key: keyGenerator()} + resolution = { + autoResolve: true, + patches: [set(newChild, [{_key: blk._key}, 'children', cIndex])], + description: `Child at index ${cIndex} is missing required _key in block with _key ${blk._key}.`, + action: 'Set a new random _key on the object', + item: blk, + + i18n: { + description: 'inputs.portable-text.invalid-value.missing-child-key.description', + action: 'inputs.portable-text.invalid-value.missing-child-key.action', + values: {key: blk._key, index: cIndex}, + }, + } + return true + } + // Verify that children have valid types if (!child._type) { resolution = { diff --git a/packages/@sanity/portable-text-editor/src/utils/values.ts b/packages/@sanity/portable-text-editor/src/utils/values.ts index 5160f57ded4..0c933e384b5 100644 --- a/packages/@sanity/portable-text-editor/src/utils/values.ts +++ b/packages/@sanity/portable-text-editor/src/utils/values.ts @@ -8,11 +8,10 @@ import { import {isEqual} from 'lodash' import {type Descendant, Element, type Node, Text} from 'slate' -import {defaultKeyGenerator as keyGenerator} from '../editor/hooks/usePortableTextEditorKeyGenerator' import {type PortableTextMemberSchemaTypes} from '../types/editor' -const EMPTY_MARKDEFS: PortableTextObject[] = [] -const EMPTY_MARKS: string[] = [] +export const EMPTY_MARKDEFS: PortableTextObject[] = [] +export const EMPTY_MARKS: string[] = [] export const VOID_CHILD_KEY = 'void-child' @@ -49,38 +48,29 @@ export function toSlateValue( const hasMissingMarkDefs = typeof textBlock.markDefs === 'undefined' const hasMissingChildren = typeof textBlock.children === 'undefined' - const children = hasMissingChildren - ? [{_type: schemaTypes.span.name, _key: keyGenerator(), text: '', marks: []}] - : textBlock.children.map((child) => { - const {_type: cType, _key: cKey, ...cRest} = child - // Return 'slate' version of inline object where the actual - // value is stored in the `value` property. - // In slate, inline objects are represented as regular - // children with actual text node in order to be able to - // be selected the same way as the rest of the (text) content. - if (cType !== 'span') { - hasInlines = true - return keepObjectEquality( - { - _type: cType, - _key: cKey, - children: voidChildren, - value: cRest, - __inline: true, - }, - keyMap, - ) - } - // If this is a span, and it's missing 'marks', add an empty marks array - if (cType === 'span' && !('marks' in cRest)) { - return keepObjectEquality( - {_key: cKey, _type: cType, ...cRest, marks: EMPTY_MARKS}, - keyMap, - ) - } - // Original child object (span) - return child - }) + const children = (textBlock.children || []).map((child) => { + const {_type: cType, _key: cKey, ...cRest} = child + // Return 'slate' version of inline object where the actual + // value is stored in the `value` property. + // In slate, inline objects are represented as regular + // children with actual text node in order to be able to + // be selected the same way as the rest of the (text) content. + if (cType !== 'span') { + hasInlines = true + return keepObjectEquality( + { + _type: cType, + _key: cKey, + children: voidChildren, + value: cRest, + __inline: true, + }, + keyMap, + ) + } + // Original child object (span) + return child + }) // Return original block if ( !hasMissingStyle && @@ -92,12 +82,10 @@ export function toSlateValue( // Original object return block } + // TODO: remove this when we have a better way to handle missing style if (hasMissingStyle) { rest.style = schemaTypes.styles[0].value } - if (hasMissingMarkDefs) { - rest.markDefs = EMPTY_MARKDEFS - } return keepObjectEquality({_type, _key, ...rest, children}, keyMap) } return keepObjectEquality( diff --git a/packages/@sanity/types/src/portableText/asserters.ts b/packages/@sanity/types/src/portableText/asserters.ts index d54653ac39b..d6c1c32e9ec 100644 --- a/packages/@sanity/types/src/portableText/asserters.ts +++ b/packages/@sanity/types/src/portableText/asserters.ts @@ -23,7 +23,7 @@ export function isPortableTextTextBlock isRecord(child)) && ('markDefs' in value // optional property ? Array.isArray(value.markDefs) && value.markDefs.every((def) => isRecord(def)) - : false) && + : true) && ('style' in value ? typeof value.style === 'string' : true) // optional property ) } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index d894834d15e..72d741a43d7 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1228,6 +1228,9 @@ importers: styled-components: specifier: ^6.1.11 version: 6.1.11(react-dom@18.3.1)(react@18.3.1) + tsx: + specifier: ^4.10.3 + version: 4.10.3 vite: specifier: ^4.5.3 version: 4.5.3(@types/node@18.19.31) @@ -12567,6 +12570,12 @@ packages: dependencies: resolve-pkg-maps: 1.0.0 + /get-tsconfig@4.7.5: + resolution: {integrity: sha512-ZCuZCnlqNzjb4QprAzXKdpp/gh6KTxSJuw3IBsPnV/7fV4NxC9ckB+vPTt8w7fJA0TaSD7c55BR47JD6MEDyDw==} + dependencies: + resolve-pkg-maps: 1.0.0 + dev: true + /get-uri@2.0.4: resolution: {integrity: sha512-v7LT/s8kVjs+Tx0ykk1I+H/rbpzkHvuIq87LmeXptcf5sNWm9uQiwjNAt94SJPA1zOlCntmnOlJvVWKmzsxG8Q==} dependencies: @@ -19449,6 +19458,17 @@ packages: /tslib@2.6.2: resolution: {integrity: sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q==} + /tsx@4.10.3: + resolution: {integrity: sha512-f0g60aFSVRVkzcQkEflh8fPLRfmt+HJHgWi/plG5UgvVaV+9TcpOwJ0sZJSACXmwmjMPg9yQR0BhTLbhkfV2uA==} + engines: {node: '>=18.0.0'} + hasBin: true + dependencies: + esbuild: 0.20.2 + get-tsconfig: 4.7.5 + optionalDependencies: + fsevents: 2.3.3 + dev: true + /tuf-js@1.1.7: resolution: {integrity: sha512-i3P9Kgw3ytjELUfpuKVDNBJvk4u5bXL6gskv572mcevPbSKCV3zt3djhmlEQ65yERjIbOSncy7U4cQJaB1CBCg==} engines: {node: ^14.17.0 || ^16.13.0 || >=18.0.0}