Skip to content

fix(text): skip initDimensions in set when a textLayoutProperty value did not change#11029

Open
jiayihu wants to merge 2 commits into
fabricjs:masterfrom
jiayihu:fix/set-initDimensions-when-value-changed
Open

fix(text): skip initDimensions in set when a textLayoutProperty value did not change#11029
jiayihu wants to merge 2 commits into
fabricjs:masterfrom
jiayihu:fix/set-initDimensions-when-value-changed

Conversation

@jiayihu

@jiayihu jiayihu commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Disclosure: I found out the issue during work but I fully asked GLM 5.2 to port the fix back to fabric, add tests and open the PR using the repo skill.

Summary

FabricText.set (and subclasses like Textbox/IText) was requesting initDimensions()/setCoords() whenever a key in textLayoutProperties was passed to set, regardless of whether the value actually changed. Setting a layout property to the same value it already held therefore triggered an unnecessary text re-measurement and coord recomputation. The fix ports the value-changed check (already used in Polyline._set) into Text.set so initDimensions is only requested when a layout-relevant value differs from the current one, while keeping the existing code path that delegates to super.set (which calls _set).

Changes

  • src/shapes/Text/Text.ts: in set, compute needsDims by comparing this[key] !== value before super.set mutates the instance (for both the string-key and object-key branches). super.set is still called for all keys, and setPathInfo() still runs after the set when a path key is present.
  • src/shapes/Text/Text.spec.ts: added two tests under a set & initDimensions describe block — one for the single-key set('text', ...) no-op vs. change case, and one for the object set({ text, fontSize, charSpacing }) no-op vs. change case — spying on initDimensions/setCoords to assert the skip behavior.

@codesandbox

codesandbox Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@github-actions

Copy link
Copy Markdown
Contributor

Build Stats

file / KB (diff) bundled minified
fabric 774.135 (+0.007) 293.395 (-0.006)

Comment thread src/shapes/Text/Text.ts
needsDims =
needsDims ||
(textLayoutProperties.includes(_key) &&
this[_key as keyof this] !== key[_key]);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@asturur

asturur commented Jun 29, 2026

Copy link
Copy Markdown
Member

I just merged a breaking change with the publishing system, i m not able to publish this, if you want this out we need to fork the current release and make a release branch.

@jiayihu

jiayihu commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

No hurry, we always directly patch or override methods at work, then I just port over the fixes to fabric when I have time

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.

2 participants