Conversation
findDOMNode is removed in React 19. HoistInputModel.domEl used it as a fallback for the case where domRef.current resolved to a class-component instance rather than a DOM element. A trace of all 25 desktop + mobile HoistInput implementations confirms every one roots domRef on a DOM element (host element, Blueprint forwardRef control, or react-onsenui's forwardRef-to-host chain), so the fallback was defensive legacy and is never exercised. domEl now resolves the element directly from the ref. Behavior-preserving on React 18; forward-compatible with React 19.
…19 prep) The legacy Blueprint Popover relies on the removed findDOMNode and breaks under React 19. PopoverNext (Floating UI based) is its React-19-compatible replacement, shipped in the same Blueprint build and running on React 18 - the same approach already used for Overlay2. The kit Popover wrapper now renders PopoverNext, converting our existing legacy PopoverProps API via Blueprint's popoverPropsToNextProps helper (maps position/modifiers/minimal/boundary and preserves the legacy shouldReturnFocusOnClose=false default). The public popover factory API is unchanged, so none of the ~20 call sites need edits. Verified no call site uses an unmappable legacy-only prop (modifiersCustom, portalStopPropagationEvents, wrapperTagName, targetClassName).
…ct 19 prep)
react-popper (Popper.js) is deprecated, its repo archived, and its peer dep
caps at React 18 - a genuine code incompatibility with React 19. The mobile
Popover was its only consumer.
Swapped usePopper for @floating-ui/react's useFloating, reusing the same
Floating UI library Blueprint's PopoverNext already pulls in (deduped to a
single 0.27.x copy) rather than adding a separate dependency. Placement names
are shared between the two libraries so menuPositionToPlacement is unchanged;
the preventOverflow modifier maps to shift({padding: 10}), 'auto' position
maps to autoPlacement(), and scroll/resize tracking uses autoUpdate.
Removed react-popper as a direct dependency (it remains transitively via
Blueprint's legacy Popover until Blueprint v7) and removed the now-obsolete
Popper.js-specific popperOptions prop from the mobile Popover.
Member
Author
|
Tested the migrated mobile Popover (Blueprint Note: mobile popovers did not appear to work on the stacked |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
React-18-safe preparation for an eventual React 19 upgrade. Every change here is forward-compatible — it behaves identically on React 18 and just removes an API/component that React 19 drops or breaks. No runtime bump, so this can merge to
developindependently of any React 19 timeline.Changes
findDOMNoderemoval —HoistInputModel.domElnow resolves its element directly fromdomRefrather than falling back to the React-19-removedfindDOMNode. A trace of all 25 desktop + mobileHoistInputimplementations confirms each already rootsdomRefon a DOM element (host element, BlueprintforwardRefcontrol, or react-onsenui's forwardRef-to-host chain), so the fallback was defensive legacy.Popover→PopoverNext— thekit/blueprintPopoverwrapper now renders Blueprint's Floating UI-basedPopoverNext(React 19 compatible) instead of the legacyfindDOMNode-basedPopover. It converts our existingPopoverPropsAPI via Blueprint'spopoverPropsToNextPropshelper (mapsposition/modifiers/minimal/boundaryand preserves the legacyshouldReturnFocusOnClosedefault), so the publicpopoverfactory API is unchanged and no call sites need edits. (Overlaywas already migrated toOverlay2.)Popover:react-popper→@floating-ui/react— swappedusePopperforuseFloating, reusing the same Floating UI library Blueprint already pulls in (deduped to a single copy rather than adding a new dependency). Removedreact-popperas a direct dependency and dropped the obsolete Popper.js-specificpopperOptionsprop.Deferred to the actual React 19 migration (not in this PR)
forwardRef→ ref-as-prop — deprecated but still works in React 19, so it's not a blocker; and ref-as-prop is React-19-only, so it can't be done while on 18.@types/react@19/ peer-range bump and the Blueprint / react-onsenui peer-depresolutionsoverrides.Testing
Verified in toolbox via
startWithHoist(React 18.2.0 / Blueprint 6.15.0): the desktop app-menu, GroupingChooser, and FilterChooser popovers plus the Select input / filter lifecycle all work with zero console errors or warnings. The mobile app builds and loads cleanly on the Floating UI swap (no module/resolution errors), but the mobile Popover's open/positioning was not exercised through the toolbox UI (its only consumer is the app-menu, which isn't surfaced in the current mobile screens) — worth a manual mobile spot-check.See
docs/planning/react-19-upgrade.mdfor the full plan. Refs #4205.