Skip to content

Commit

Permalink
fix: improve the rendering performance (#58)
Browse files Browse the repository at this point in the history
* fix: improve performance by only rendering ContextMenuButton when hovered or selected
* fix: improve performance by memoizing the paths passed to child components
  • Loading branch information
josdejong authored Apr 11, 2022
1 parent fbc7b98 commit 84c6eb3
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 25 deletions.
3 changes: 1 addition & 2 deletions src/lib/components/modes/treemode/JSONNode.scss
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
$height: 2px;
$height-half: 0.5 * $height;

display: none;
display: flex;
position: relative;

max-width: 250px;
Expand All @@ -126,7 +126,6 @@
}

&.hovered {
display: flex;
outline-color: $hovered-background-transparent;
}
}
Expand Down
23 changes: 14 additions & 9 deletions src/lib/components/modes/treemode/JSONNode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import { onMoveSelection } from '../../../logic/dragging.js'
import { forEachIndex } from '../../../utils/arrayUtils.js'
import { getDataPathFromTarget } from '../../../utils/domUtils.js'
import { createMemoizePath } from '../../../utils/pathUtils.js'
export let value
export let path
Expand Down Expand Up @@ -86,6 +87,12 @@
return `margin-left: ${level * INDENTATION_WIDTH}px`
}
$: indentationStyle = getIndentationStyle(path.length)
// important to prevent creating a new path for all children with every re-render,
// that would force all childs to re-render
const memoizePath = createMemoizePath()
function toggleExpand(event) {
event.stopPropagation()
Expand Down Expand Up @@ -442,8 +449,6 @@
context.onSelect({ type: SELECTION_TYPE.AFTER, path })
context.onContextMenu(props)
}
$: indentationStyle = getIndentationStyle(path.length)
</script>
<div
Expand Down Expand Up @@ -521,7 +526,7 @@
</div>
{#if expanded}
<div class="items">
{#if !context.readOnly}
{#if !context.readOnly && (hover === HOVER_INSERT_INSIDE || selectedInside)}
<div
class="insert-area inside"
class:hovered={hover === HOVER_INSERT_INSIDE}
Expand All @@ -537,10 +542,10 @@
</div>
{/if}
{#each visibleSections as visibleSection, sectionIndex (sectionIndex)}
{#each resolvedValue.slice(visibleSection.start, Math.min(visibleSection.end, resolvedValue.length)) as item, itemIndex (resolvedState[visibleSection.start + itemIndex][STATE_ID])}
{#each resolvedValue.slice(visibleSection.start, Math.min(visibleSection.end, resolvedValue.length)) as item, itemIndex (itemIndex)}
<svelte:self
value={item}
path={path.concat(visibleSection.start + itemIndex)}
path={memoizePath(path.concat(visibleSection.start + itemIndex))}
state={resolvedState[visibleSection.start + itemIndex]}
selection={resolvedSelection
? resolvedSelection[visibleSection.start + itemIndex]
Expand Down Expand Up @@ -641,7 +646,7 @@
</div>
{#if expanded}
<div class="props">
{#if !context.readOnly}
{#if !context.readOnly && (hover === HOVER_INSERT_INSIDE || selectedInside)}
<div
class="insert-area inside"
class:hovered={hover === HOVER_INSERT_INSIDE}
Expand All @@ -659,7 +664,7 @@
{#each keys as key (resolvedState[key][STATE_ID])}
<svelte:self
value={resolvedValue[key]}
path={path.concat(key)}
path={memoizePath(path.concat(key))}
state={resolvedState[key]}
selection={resolvedSelection ? resolvedSelection[key] : undefined}
searchResult={searchResult ? searchResult[key] : undefined}
Expand All @@ -669,7 +674,7 @@
>
<div slot="identifier" class="identifier">
<JSONKey
path={path.concat(key)}
path={memoizePath(path.concat(key))}
{key}
{context}
selection={resolvedSelection?.[key]?.[STATE_SELECTION]}
Expand Down Expand Up @@ -729,7 +734,7 @@
{/if}
</div>
{/if}
{#if !context.readOnly}
{#if !context.readOnly && (hover === HOVER_INSERT_AFTER || selectedAfter)}
<div
class="insert-area after"
class:hovered={hover === HOVER_INSERT_AFTER}
Expand Down
3 changes: 2 additions & 1 deletion src/lib/components/modes/treemode/TreeMode.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@
let json
let text
let state = syncState({}, undefined, [], expandMinimal)
const rootPath = [] // create the array only once
let selection = null
Expand Down Expand Up @@ -2050,7 +2051,7 @@
<div class="contents" data-jsoneditor-scrollable-contents={true} bind:this={refContents}>
<JSONNode
value={json}
path={[]}
path={rootPath}
{state}
selection={recursiveSelection}
searchResult={searchResult && searchResult.itemsWithActive}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@

.insert-area {
&.selected {
display: flex;
z-index: 2;
outline-color: $insert-area-background;
}
Expand Down
12 changes: 12 additions & 0 deletions src/lib/utils/pathUtils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { memoize } from 'lodash-es'

/**
* Stringify a path like
*
Expand All @@ -20,3 +22,13 @@ export function stringifyPath(path) {
})
.join('')
}

/**
* Create a memoized function that will memoize the input path, and return
* the memoized instance of the path when the stringified version is the same.
*
* @returns {(path: Path) => Path}
*/
export function createMemoizePath() {
return memoize((path) => path, stringifyPath)
}
40 changes: 28 additions & 12 deletions src/lib/utils/pathUtils.test.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,33 @@
import assert from 'assert'
import { stringifyPath } from './pathUtils.js'
import { notStrictEqual, strictEqual } from 'assert'
import { createMemoizePath, stringifyPath } from './pathUtils.js'

describe('pathUtils', () => {
it('stringifyPath', () => {
assert.strictEqual(stringifyPath([]), '')
assert.strictEqual(stringifyPath(['']), '[""]')
assert.strictEqual(stringifyPath(['foo']), '.foo')
assert.strictEqual(stringifyPath(['foo', 'bar']), '.foo.bar')
assert.strictEqual(stringifyPath(['foo', 2]), '.foo[2]')
assert.strictEqual(stringifyPath(['foo', 2, 'bar']), '.foo[2].bar')
assert.strictEqual(stringifyPath(['foo', 2, 'bar_baz']), '.foo[2].bar_baz')
assert.strictEqual(stringifyPath([2]), '[2]')
assert.strictEqual(stringifyPath(['foo', 'prop-with-hyphens']), '.foo["prop-with-hyphens"]')
assert.strictEqual(stringifyPath(['foo', 'prop with spaces']), '.foo["prop with spaces"]')
strictEqual(stringifyPath([]), '')
strictEqual(stringifyPath(['']), '[""]')
strictEqual(stringifyPath(['foo']), '.foo')
strictEqual(stringifyPath(['foo', 'bar']), '.foo.bar')
strictEqual(stringifyPath(['foo', 2]), '.foo[2]')
strictEqual(stringifyPath(['foo', 2, 'bar']), '.foo[2].bar')
strictEqual(stringifyPath(['foo', 2, 'bar_baz']), '.foo[2].bar_baz')
strictEqual(stringifyPath([2]), '[2]')
strictEqual(stringifyPath(['foo', 'prop-with-hyphens']), '.foo["prop-with-hyphens"]')
strictEqual(stringifyPath(['foo', 'prop with spaces']), '.foo["prop with spaces"]')
})

it('createMemoizePath', () => {
const memoizePath = createMemoizePath()

const path1 = ['a', 'b']
strictEqual(memoizePath(path1), path1)

const path2 = path1.slice(0)
strictEqual(memoizePath(path2), path1)

const memoizePath2 = createMemoizePath()
const path3 = path1.slice(0)
strictEqual(memoizePath2(path3), path3) // must NOT be path1, must be isolated from memoizePath

notStrictEqual(path1, path1.slice(0)) // just to validate that strictEqual works the way I think
})
})

0 comments on commit 84c6eb3

Please sign in to comment.