[diogoperes]

On our client app we are reusing some text elements. To avoid always doing xProperty = config.x || defaulttX it would be simpler to allow undefined on the setter functions.

Some properties already seem to assume the value can be undefined, despite the type being mandatory, like the letterSpacing or the text setter property. This only passes because the eslint rule @typescript-eslint/no-unnecessary-condition is turned off.

Summary

• Text setters on CoreTextNode now use the same defaults as createTextNode when the value is undefined.
• 0 and '' are kept. createTextNode still uses ||, so a falsy value passed at creation is still replaced.
• text still turns null into '', and non-strings such as 0 and false into '0' and 'false'.
• maxWidth, maxHeight, and forceLoad are unchanged.
• Canvas text reads letterSpacing directly. Creation already resolves it.

Test plan

• pnpm vitest run src/core/CoreTextNode.test.ts
• pnpm start, open /?test=text-align, and check left, center, and right

[wouterlucas]

nice! this looks good - want to add more or can we mark it ready for review?

[diogoperes]

Thanks for the feedback. I think it would also be nice to do this for the CoreNode setters as a future work. but for now, to decrease the amount of changes I'm just changing the CoreTextNode.
I just opened this as draft to get some feedback, but since you agree with the proposed change I will put this as ready for review.

[wouterlucas]

LGTM @jfboeve can you do a final scan and merge it if you think its good?

[jfboeve]

What is this change for?

[diogoperes]

My bad, I should have reverted that after testing, currently we have a patch on our client repo to change that property to protected because we are extending the CoreTextNode class and we have custom setters where we are doing: this._layoutGenerated = false;.

This an unrelated change so I will revert that, well spotted

[diogoperes]

Done on: dbc6294