Skip to content

fix(core): skip full-tree serialization when onNodesChange is not provided - #752

Open
hnldlsjzt wants to merge 1 commit into
prevwong:mainfrom
hnldlsjzt:fix/skip-serialize-without-onnodeschange
Open

hnldlsjzt wants to merge 1 commit into
prevwong:mainfrom
hnldlsjzt:fix/skip-serialize-without-onnodeschange

Conversation

@hnldlsjzt

Copy link
Copy Markdown

Problem

<Editor> unconditionally subscribes with a collector that calls query.serialize() — a JSON.stringify of the entire node tree — on every store change, solely to detect whether the onNodesChange callback should fire:

https://github.com/prevwong/craft.js/blob/master/packages/core/src/editor/Editor.tsx#L94-L103

Since onNodesChange defaults to a no-op, consumers that never pass it still pay this serialization cost on every state change.

Real-world impact

We render read-only pages (enabled: false) that receive frequent runtime prop updates over WebSocket (industrial monitoring dashboards running 24/7). On a page with ~500 nodes (~450 KB serialized JSON), CPU profiling showed:

  • this serialization was the single largest JS cost (~20% of busy time);
  • it was a major source of GC pressure — a ~450 KB string allocated and immediately discarded on every incoming data update.

With this fix applied (verified via pnpm patch in production), overall busy time on a 20-second profile dropped from 11.8% to 5.5%, with the serialize cost going to zero.

Fix

Only set up the subscription when the consumer explicitly provides onNodesChange. Consumers that pass the callback (i.e. editors) are unaffected — the subscription is created exactly as before. The guard also protects against context being null, consistent with the other effects in the component.

No API changes, no behavior changes for existing onNodesChange users.

…vided

The Editor component unconditionally subscribes with a collector that
calls query.serialize() (a JSON.stringify of the entire node tree) on
every store change, solely to detect whether the onNodesChange callback
should fire.

For consumers that never pass onNodesChange - typically read-only
viewers/renderers that receive frequent runtime prop updates - this
serialization is pure overhead: on a page with ~500 nodes (~450KB of
serialized JSON) every incoming data update paid a full-tree stringify,
which profiling showed to be the single largest JS cost and a major
source of GC pressure.

Only set up the subscription when the consumer explicitly provides
onNodesChange. Editors that pass the callback are unaffected.
@hnldlsjzt
hnldlsjzt requested a review from prevwong as a code owner July 29, 2026 08:45
@vercel

vercel Bot commented Jul 29, 2026

Copy link
Copy Markdown

@hnldlsjzt is attempting to deploy a commit to the Prev Wong's projects Team on Vercel.

A member of the Team first needs to authorize it.

@changeset-bot

changeset-bot Bot commented Jul 29, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 7bdf748

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant