Skip to content

Add React 18 support to @logicsoftware/slider - #8

Draft
paul1k with Copilot wants to merge 4 commits into
masterfrom
copilot/add-react-18-support
Draft

paul1k with Copilot wants to merge 4 commits into
masterfrom
copilot/add-react-18-support

Conversation

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown

Upgrades the custom @logicsoftware/slider fork to support React 18 while preserving all custom logic. Targets deprecated lifecycle methods, removed APIs, and test infrastructure incompatibilities. Released as version 9.0.0 (major version bump reflecting the breaking upgrade).

Dependency updates (package.json)

  • version: bumped to 9.0.0
  • peerDependencies: now accepts React ^16 || ^17 || ^18
  • devDependencies: React/ReactDOM/react-test-renderer → ^18, Jest ^20^27.5.1, enzyme-adapter-react-16@cfaester/enzyme-adapter-react-18@^0.8.0, added jest-environment-jsdom (Jest 27 changed default testEnvironment to node)

Lifecycle migration (src/Slider.jsx, src/Range.jsx)

Replaced componentWillReceiveProps with the React 18-safe pattern:

// Synchronous state clamping during render (replaces the setState call in cWRP)
static getDerivedStateFromProps(props, state) { ... }

// Side-effect only: fire onChange when value was out of range before clamping
componentDidUpdate(prevProps, prevState) {
  // Must use prevState.value — getDerivedStateFromProps has already clamped
  // this.state.value by the time componentDidUpdate runs
  const value = this.props.value !== undefined ? this.props.value : prevState.value;
  if (utils.isValueOutOfRange(value, this.props)) {
    this.props.onChange(this.trimAlignValue(value));
  }
}

Range.jsx follows the same pattern with prevState.bounds.

findDOMNode removal (src/utils.js)

Replaced findDOMNode(handles[key]) with handles[key] && handles[key].handleHandle already stores its root DOM node as this.handle via a ref callback.

Examples (examples/*.js)

All 6 files migrated from ReactDOM.render to createRoot().render().

Test infrastructure

  • tests/setup.js: New adapter, TextEncoder polyfill, and a critical patch for Enzyme 3.x + React 18: Enzyme.ReactWrapper.prototype.setState is wrapped with ReactDOM.flushSync because Enzyme 3 calls instance.setState() directly without act(), causing React 18's concurrent scheduler to defer updates past test assertions.
  • tests/common/createSlider.test.js: Direct wrapper.instance().onMouseMove() calls wrapped in act() — outside React's event system, React 18 batches these state updates.
  • tests/Range.test.js: jest.genMockFn()jest.fn() (removed in Jest 27); map.mousemove()/map.mouseup() direct listener calls wrapped in act().

Pre-existing failures (not introduced here, not fixed)

  • 4× touch-event tests — createSlider.jsx has no onTouchStart / onTouchUpListener implementation
  • Range › focus & bluronFocus/onBlur props are never forwarded into the Handle's {...restProps}, so programmatic .focus() does not trigger the callback

@paul1k

paul1k commented Jun 29, 2026

Copy link
Copy Markdown

@copilot plz increase package version

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Author

@copilot plz increase package version

Done — bumped version from 8.6.20 to 8.7.0 in commit chore: bump version to 8.7.0 for React 18 support.

@paul1k

paul1k commented Jun 29, 2026

Copy link
Copy Markdown

@copilot plz increase package version

Done — bumped version from 8.6.20 to 8.7.0 in commit chore: bump version to 8.7.0 for React 18 support.

@copilot plz increase major version

Copilot AI commented Jun 29, 2026

Copy link
Copy Markdown
Author

@copilot plz increase package version

Done — bumped version from 8.6.20 to 8.7.0 in commit chore: bump version to 8.7.0 for React 18 support.
...

Done — version is now 9.0.0 (committed in chore: bump major version to 9.0.0 for React 18 support).

@paul1k

paul1k commented Jul 5, 2026

Copy link
Copy Markdown

@copilot plz check test and fix

- createSlider.jsx: add onTouchStart, onTouchMove, addDocumentTouchEvents;
  extend removeDocumentEvents to clean up touch listeners; wire
  onTouchStart to the slider div
- Range.jsx, Slider.jsx: forward onFocus/onBlur props to the handle
  generator so they reach the Handle DOM element
- Handle.jsx: explicitly extract onFocus/onBlur from props; apply
  onFocus directly on the div and merge onBlur with the internal
  handleBlur so both the internal state update and the prop callback run
- tests/common/createSlider.test.js: wrap direct onTouchMove call in
  act() + wrapper.update() (same pattern as onMouseMove) so React 18
  batched state updates are flushed before assertions

Copilot AI commented Jul 5, 2026

Copy link
Copy Markdown
Author

@copilot plz check test and fix

Fixed all 6 failing tests in the latest commit. Here's what was done:

Touch-event tests (4 fixes) — src/common/createSlider.jsx

  • Added onTouchStart handler (mirrors onMouseDown: computes dragOffset, calls onStart, attaches document listeners)
  • Added onTouchMove handler (mirrors onMouseMove: calls onMove with position minus dragOffset)
  • Added addDocumentTouchEvents() which sets this.onTouchUpListener (touchend → onEnd) and this.onTouchMoveListener
  • Extended removeDocumentEvents() to clean up both touch listeners
  • Wired onTouchStart to the slider div

Range focus/blur tests (2 fixes)

  • Range.jsx / Slider.jsx: forward onFocus/onBlur props from this.props to the handle generator
  • Handle.jsx: explicitly extract onFocus/onBlur from props; apply onFocus directly on the div; merge onBlur with the internal handleBlur so the click-focus state is still cleared AND the prop callback fires

Test fix — tests/common/createSlider.test.js

  • Wrapped the direct onTouchMove() call in act() + wrapper.update() (same pattern already applied to onMouseMove) so React 18's batched state updates are flushed before assertions

All 5 suites now pass: 36 passed, 15 skipped, 0 failed.

Copilot finished work on behalf of paul1k July 5, 2026 10:33
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