Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Top-level deletions and insertions can hang entire editor #37

Open
mkremins opened this issue Sep 14, 2014 · 2 comments
Open

Top-level deletions and insertions can hang entire editor #37

mkremins opened this issue Sep 14, 2014 · 2 comments

Comments

@mkremins
Copy link
Owner

Specifically, there's a performance problem with inserting and deleting top-level forms early in the document when the document contains any nontrivial number of top-level forms. Top-level insertions and deletions seem to force React to re-render every top-level form following the one that was deleted or inserted, hammering the DOM with way more changes than necessary.

The most obvious solution: assign a unique React key to every parse tree node, such that React will know to reorder the list of top-level forms slightly (rather than re-rendering the majority of forms in the list) when an insertion or deletion takes place. This will presumably also solve potential future performance problems involving insertions and deletions within particularly large subtrees – problems that I haven't yet witnessed, but that would logically appear under sufficiently pathological conditions.

@mkremins mkremins added the bug label Sep 14, 2014
@mkremins mkremins changed the title Top-level deletions and insertions can cause editor to hang Top-level deletions and insertions can hang entire editor Sep 14, 2014
@mkremins
Copy link
Owner Author

On second thought, this wouldn't necessarily fix all the potential future perf issues within subtrees – only top-level forms correspond directly to parse tree nodes, so we'd have to give a unique and predictable React key to each generated token if we wanted to take full advantage of React's sibling reordering algorithm at a subtree level.

That said, the fix discussed here should still work for top-level forms, which is where the major problems we're seeing right now actually are.

@mkremins
Copy link
Owner Author

Tried my hand at a naïve implementation of the fix discussed above, assigning a monotonically increasing integer :id to each parse tree node upon creation and using this ID as the React key for each top-level form. Unfortunately it didn't seem to fix the problem; both deletion and insertion early in the document remained slow, suggesting that the problem may not be as straightforward as I'd thought.

Notes on React's list-wise diff algorithm are available here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant