Skip to content

Commit

Permalink
fix: let text mode not change json contents directly into text cont…
Browse files Browse the repository at this point in the history
…ents, and prevent freezing when loading a large document (#141)

* fix: keep json content in TextMode instead of emitting the contents changed to text (WIP)

* fix: disable text editor when opening a large file

* chore: some refactoring, return early instead of nesting if statements

* chore: remove `npm test` from pre-commit

* chore: initialize editorDisabled directly

* fix: accepting too large document not working
  • Loading branch information
josdejong committed Sep 12, 2022
1 parent 5e7790e commit 28b2b56
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 59 deletions.
1 change: 0 additions & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npm test
npm run lint
6 changes: 1 addition & 5 deletions src/lib/components/JSONEditor.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,6 @@
return content
}
function getText(content: Content) {
return isTextContent(content) ? content.text : JSON.stringify(content.json, null, indentation)
}
export function set(newContent: Content) {
debug('set')
Expand Down Expand Up @@ -407,7 +403,7 @@
{#if mode === Mode.text || mode === 'code'}
<TextMode
bind:this={refTextMode}
text={getText(content)}
externalContent={content}
{readOnly}
{indentation}
{tabSize}
Expand Down
129 changes: 76 additions & 53 deletions src/lib/components/modes/textmode/TextMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import type { JSONPatchDocument } from 'immutable-json-patch'
import { immutableJSONPatch, revertJSONPatch } from 'immutable-json-patch'
import jsonrepair from 'jsonrepair'
import { debounce, noop, uniqueId } from 'lodash-es'
import { debounce, isEqual, noop, uniqueId } from 'lodash-es'
import { onDestroy, onMount } from 'svelte'
import {
JSON_STATUS_INVALID,
Expand All @@ -21,7 +21,7 @@
getWindow
} from '$lib/utils/domUtils'
import { formatSize } from '$lib/utils/fileUtils'
import { findTextLocation } from '$lib/utils/jsonUtils'
import { findTextLocation, getText } from '$lib/utils/jsonUtils'
import { createFocusTracker } from '../../controls/createFocusTracker.js'
import Message from '../../controls/Message.svelte'
import ValidationErrorsOverview from '../../controls/ValidationErrorsOverview.svelte'
Expand All @@ -39,6 +39,7 @@
import StatusBar from './StatusBar.svelte'
import { highlighter } from './codemirror/codemirror-theme'
import type {
Content,
ContentErrors,
JSONPatchResult,
OnChange,
Expand All @@ -55,7 +56,7 @@
export let readOnly = false
export let mainMenuBar = true
export let statusBar = true
export let text = ''
export let externalContent: Content
export let indentation: number | string = 2
export let tabSize = 4
export let escapeUnicodeCharacters = false
Expand Down Expand Up @@ -83,7 +84,6 @@
let codeMirrorRef
let codeMirrorView
let codeMirrorText
let domTextMode
let editorState: EditorState
Expand All @@ -96,16 +96,17 @@
const indentUnitCompartment = new Compartment()
const tabSizeCompartment = new Compartment()
let content: Content = externalContent
let text = getText(content, indentation) // text is just a cached version of content.text or parsed content.json
let editorDisabled = disableTextEditor(text, acceptTooLarge)
$: isNewDocument = text.length === 0
$: tooLarge = text && text.length > MAX_DOCUMENT_SIZE_TEXT_MODE
$: textEditorDisabled = tooLarge && !acceptTooLarge
$: normalization = createNormalizationFunctions({
escapeControlCharacters: false,
escapeUnicodeCharacters
})
$: setCodeMirrorValue(text)
$: setCodeMirrorContent(externalContent)
$: updateLinter(validator)
$: updateIndentation(indentation)
$: updateTabSize(tabSize)
Expand All @@ -126,11 +127,9 @@
}
try {
codeMirrorText = !textEditorDisabled ? text : ''
codeMirrorView = createCodeMirrorView({
target: codeMirrorRef,
initialText: codeMirrorText,
initialText: !editorDisabled ? text : '',
readOnly,
indentation
})
Expand Down Expand Up @@ -181,7 +180,9 @@
const previousJson = JSON.parse(text)
const updatedJson = immutableJSONPatch(previousJson, operations)
const undo = revertJSONPatch(previousJson, operations)
text = JSON.stringify(updatedJson, null, indentation)
setCodeMirrorContent({
text: JSON.stringify(updatedJson, null, indentation)
})
return {
json: updatedJson,
Expand All @@ -200,7 +201,9 @@
try {
const json = JSON.parse(text)
text = JSON.stringify(json, null, indentation)
setCodeMirrorContent({
text: JSON.stringify(json, null, indentation)
})
} catch (err) {
onError(err)
}
Expand All @@ -215,7 +218,9 @@
try {
const json = JSON.parse(text)
text = JSON.stringify(json)
setCodeMirrorContent({
text: JSON.stringify(json)
})
} catch (err) {
onError(err)
}
Expand All @@ -229,7 +234,9 @@
}
try {
text = jsonrepair(text)
setCodeMirrorContent({
text: jsonrepair(text)
})
jsonStatus = JSON_STATUS_VALID
jsonParseError = undefined
} catch (err) {
Expand Down Expand Up @@ -353,7 +360,7 @@
function handleAcceptTooLarge() {
acceptTooLarge = true
setCodeMirrorValue(text, true)
setCodeMirrorContent(externalContent, true)
}
function cancelLoadTooLarge() {
Expand Down Expand Up @@ -529,31 +536,38 @@
}
}
function setCodeMirrorValue(text, force = false) {
if (textEditorDisabled && !force) {
debug('not applying text: editor is disabled')
function setCodeMirrorContent(newContent: Content, forceUpdate = false) {
const newText = getText(newContent, indentation)
editorDisabled = disableTextEditor(newText, acceptTooLarge)
if (editorDisabled) {
debug('externalContent not applying text: editor is disabled')
return
}
const isChanged = text !== codeMirrorText
debug('setCodeMirrorValue', { isChanged, length: text.length })
const isChanged = !isEqual(newContent, content)
debug('setCodeMirrorContent', { isChanged, forceUpdate })
if (!codeMirrorView || (!isChanged && !forceUpdate)) {
return
}
if (codeMirrorView && isChanged) {
const previousText = codeMirrorText
codeMirrorText = text
const previousContent = content
content = newContent
text = newText
// keep state
// to reset state: codeMirrorView.setState(EditorState.create({doc: text, extensions: ...}))
codeMirrorView.dispatch({
changes: {
from: 0,
to: codeMirrorView.state.doc.length,
insert: normalization.escapeValue(text)
}
})
// keep state
// to reset state: codeMirrorView.setState(EditorState.create({doc: text, extensions: ...}))
codeMirrorView.dispatch({
changes: {
from: 0,
to: codeMirrorView.state.doc.length,
insert: normalization.escapeValue(text)
}
})
updateCanUndoRedo()
emitOnChange(text, previousText)
updateCanUndoRedo()
if (isChanged) {
emitOnChange(content, previousContent)
}
}
Expand Down Expand Up @@ -594,18 +608,20 @@
return
}
codeMirrorText = getCodeMirrorValue()
const codeMirrorText = getCodeMirrorValue()
const isChanged = codeMirrorText !== text
debug('onChangeCodeMirrorValue', { isChanged })
if (!isChanged) {
return
}
if (isChanged) {
const previousText = text
text = codeMirrorText
const previousContent = content
text = codeMirrorText
content = { text }
updateCanUndoRedo()
emitOnChange(text, previousText)
}
updateCanUndoRedo()
emitOnChange(content, previousContent)
}
function updateLinter(validator) {
Expand Down Expand Up @@ -672,24 +688,29 @@
TEXT_MODE_ONCHANGE_DELAY
)
function emitOnChange(text: string, previousText: string) {
function emitOnChange(content: Content, previousContent: Content) {
if (onChange) {
onChange(
{ text },
{ text: previousText },
{
contentErrors: validate(),
patchResult: null
}
)
onChange(content, previousContent, {
contentErrors: validate(),
patchResult: null
})
}
}
function disableTextEditor(text: string, acceptTooLarge: boolean): boolean {
const tooLarge = text && text.length > MAX_DOCUMENT_SIZE_TEXT_MODE
return tooLarge && !acceptTooLarge
}
let jsonStatus = JSON_STATUS_VALID
let jsonParseError: ParseError | null = null
function linterCallback(): Diagnostic[] {
if (editorDisabled) {
return []
}
const contentErrors = validate()
if (isContentParseError(contentErrors)) {
Expand All @@ -706,7 +727,7 @@
}
export function validate(): ContentErrors {
debug('validate')
debug('validate:start')
onChangeCodeMirrorValueDebounced.flush()
Expand All @@ -722,6 +743,8 @@
validationErrors = contentErrors.validationErrors
}
debug('validate:end')
return contentErrors
}
Expand Down Expand Up @@ -764,7 +787,7 @@
{/if}

{#if !isSSR}
{#if textEditorDisabled}
{#if editorDisabled}
<Message
icon={faExclamationTriangle}
type="error"
Expand Down Expand Up @@ -796,7 +819,7 @@
/>
{/if}

<div class="jse-contents" class:jse-hidden={textEditorDisabled} bind:this={codeMirrorRef} />
<div class="jse-contents" class:jse-hidden={editorDisabled} bind:this={codeMirrorRef} />

{#if statusBar}
<StatusBar {editorState} />
Expand Down
7 changes: 7 additions & 0 deletions src/lib/utils/jsonUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,13 @@ export function isTextContent(content: Content): content is TextContent {
return typeof (content as Record<string, unknown>).text === 'string'
}

/**
* Get the contents as Text. If the contents is JSON, the JSON will be parsed.
*/
export function getText(content: Content, indentation: number | string) {
return isTextContent(content) ? content.text : JSON.stringify(content.json, null, indentation)
}

/**
* Returns true when the (estimated) size of the contents exceeds the
* provided maxSize.
Expand Down

0 comments on commit 28b2b56

Please sign in to comment.