NXT-10864 [Enact] Spottable - Investigate performance improvements

[dan-ichim-lgp]

Checklist

• I have read and understand the contribution guide
• A CHANGELOG entry is included
• At least one test case is included for this feature or bug fix
• Documentation was added or is not needed
• This is an API breaking change

Issue Resolved / Feature Added

Reduced per-instance render overhead in spotlight/Spottable, with particular impact in VirtualList scrolling where many spottable items mount and re-render frequently.

Resolution

Five independent optimizations:

Removed redundant mount render (Spottable.js) — componentDidMount called forceUpdate(), a leftover from the ReactDOM.findDOMNode() era. With WithRef + useImperativeHandle, the DOM node is available before useLayoutEffect fires on the first render, making the second render unnecessary. Saves one full render per Spottable mount.

Converted Spottable class wrapper to a function component (Spottable.js) — the inner Spottable class existed only to hold a handleForceUpdate instance method. It is now a plain function component that calls useReducer for a stable, allocation-free force-update trigger, removing a class instance and its field allocation per mount.

Removed dead delete rest.spotlightId (Spottable.js) — spotlightId is already destructured out of props in SpottableBase, so it is never present in rest. The delete was a no-op on every render.

Conditional Spotlight.getCurrent() (SpottableCore.js) — didUpdate previously called getCurrent() on every render to re-sync this.isFocused. The call is now skipped unless spotlightDisabled is true or a selection cancel may fire. this.isFocused stays accurate via handleFocus/handleBlur events in all other cases. unload() re-queries getCurrent() directly at unmount to keep onSpotlightDisappear reliable.

Reduced per-render allocations (useSpottable.js) — three sources of unnecessary allocations eliminated:

context.current is now mutated in-place instead of being replaced with a new object each render.
A shared EMPTY_ATTRIBUTES constant is used when spotlightId is not set, avoiding an empty-object allocation on every render.
The props rest object (a fresh copy already created by destructuring) is mutated in-place before being passed to hook.setPropsAndContext, avoiding a second spread-merge allocation that previously occurred on every render.

Additional Considerations

No API or behavioral changes. Render-count expectations in shouldComponentUpdate tests updated to reflect the removed mount-time forceUpdate().

Links

NXT-10864
Related: findDOMNode removal — commit 33c27d5 (WRQ-19325)

Comments

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.53%. Comparing base (5a9fba1) to head (6ee9f2f).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3402      +/-   ##
===========================================
+ Coverage    84.37%   84.53%   +0.16%     
===========================================
  Files          154      154              
  Lines         7455     7463       +8     
  Branches      1968     1974       +6     
===========================================
+ Hits          6290     6309      +19     
+ Misses         918      910       -8     
+ Partials       247      244       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[daniel-stoian-lgp]

Looks ok, but i can suggest some improvements:

1 Spottable.js:

line 153: Dead delete rest.spotlightId : spotlightId is already destructured out of props into its own variable at the top of SpottableBase, so it's never in rest to begin with

2. Spottable.js
   This class component can be made functional:

class Spottable extends Component {
handleForceUpdate = () => {
this.forceUpdate();
};

  render () {
  	return <SpottableBase {...this.props} handleForceUpdate={this.handleForceUpdate} />;
  }

}

functional:

function Spottable (props) {
// The outer Spottable class exists for one reason only — to produce a stable handleForceUpdate callback.
// useReducer's dispatch is guaranteed stable across renders by React, so it can replace the class entirely:
const forceUpdate = useReducer(n => n + 1, 0)[1];
return <SpottableBase {...props} handleForceUpdate={forceUpdate} />;
}

3. useSpottable.js: Double object allocation on every render

// Object 1: created by destructuring rest
const useSpottable = ({emulateMouse, getSpotRef, selectionKeys = [...], spotlightDisabled, ...props} = {}) => {
// ...
// Object 2: created here to merge them back together
hook.setPropsAndContext({selectionKeys, spotlightDisabled, ...props}, context.current);

Object 2 can be modified to:

props.selectionKeys = selectionKeys;
props.spotlightDisabled = spotlightDisabled;
hook.setPropsAndContext(props, context.current);

Since props (the rest object) is already a fresh copy, it can just be mutated in-place before passing

Please test and let me know your opinion about the suggestions

[daniel-stoian-lgp]

please add information about the changes made after my comments, in the PR description

[alexandrumorariu]

LGTM