feat!: bump react-overlays (#5017)

* feat: update to popper v2 BREAKING CHANGE: only breaking if you pass a popperConfig directly to overlay or dropdown components. The config format is now the v2 one. This change only relevant to users of `popperConfig` prop on Overlay, OverlayTrigger, and Dropdown * docs: update some packages, clean up version dropdown a bit * Apply suggestions from code review * fail in the right place * clean up popper injected props * fix types Co-authored-by: Jimmy Jia <tesrin@gmail.com>
react-bootstrap · Mar 16, 2020 · 1b20f1b · 1b20f1b
1 parent 724ea6f
commit 1b20f1b
Show file tree

Hide file tree

Showing 19 changed files with 239 additions and 117 deletions.
diff --git a/package.json b/package.json
@@ -66,14 +66,13 @@
     "@babel/runtime": "^7.4.2",
     "@restart/context": "^2.1.4",
     "@restart/hooks": "^0.3.21",
-    "@types/react": "^16.8.23",
+    "@types/react": "^16.9.23",
     "classnames": "^2.2.6",
     "dom-helpers": "^5.1.2",
     "invariant": "^2.2.4",
-    "popper.js": "^1.16.0",
     "prop-types": "^15.7.2",
     "prop-types-extra": "^1.1.0",
-    "react-overlays": "^2.1.0",
+    "react-overlays": "^3.0.1",
     "react-transition-group": "^4.0.0",
     "uncontrollable": "^7.0.0",
     "warning": "^4.0.3"

diff --git a/src/Dropdown.js b/src/Dropdown.js
@@ -35,7 +35,7 @@ const propTypes = {
 
   /**
    * Allow Dropdown to flip in case of an overlapping on the reference element. For more information refer to
-   * Popper.js's flip [docs](https://popper.js.org/popper-documentation.html#modifiers..flip.enabled).
+   * Popper.js's flip [docs](https://popper.js.org/docs/v2/modifiers/flip/).
    *
    */
   flip: PropTypes.bool,

diff --git a/src/DropdownMenu.js b/src/DropdownMenu.js
@@ -43,7 +43,7 @@ const propTypes = {
   as: PropTypes.elementType,
 
   /**
-   * A set of popper options and props passed directly to react-popper's Popper component.
+   * A set of popper options and props passed directly to Popper.
    */
   popperConfig: PropTypes.object,
 };

diff --git a/src/Overlay.js b/src/Overlay.js
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useRef } from 'react';
 import { findDOMNode } from 'react-dom';
 import classNames from 'classnames';
 import PropTypes from 'prop-types';
@@ -26,7 +26,7 @@ const propTypes = {
   show: PropTypes.bool,
 
   /**
-   * A set of popper options and props passed directly to react-popper's Popper component.
+   * A set of popper options and props passed directly to Popper.
    */
   popperConfig: PropTypes.object,
 
@@ -121,25 +121,45 @@ function wrapRefs(props, arrowProps) {
 }
 
 function Overlay({ children: overlay, transition, ...outerProps }) {
+  const popperRef = useRef({});
   transition = transition === true ? Fade : transition || null;
 
   return (
     <BaseOverlay {...outerProps} transition={transition}>
-      {({ props: overlayProps, arrowProps, show, ...props }) => {
+      {({
+        props: overlayProps,
+        arrowProps,
+        show,
+        state,
+        scheduleUpdate,
+        placement,
+        outOfBoundaries,
+        ...props
+      }) => {
         wrapRefs(overlayProps, arrowProps);
+        const popper = Object.assign(popperRef.current, {
+          state,
+          scheduleUpdate,
+          placement,
+          outOfBoundaries,
+        });
 
         if (typeof overlay === 'function')
           return overlay({
             ...props,
             ...overlayProps,
+            placement,
             show,
+            popper,
             arrowProps,
           });
 
         return React.cloneElement(overlay, {
           ...props,
           ...overlayProps,
+          placement,
           arrowProps,
+          popper,
           className: classNames(
             overlay.props.className,
             !transition && show && 'show',

diff --git a/src/OverlayTrigger.js b/src/OverlayTrigger.js
@@ -192,19 +192,23 @@ function OverlayTrigger({
   // We add aria-describedby in the case where the overlay is a role="tooltip"
   // for other cases describedby isn't appropriate (e.g. a popover with inputs) so we don't add it.
   const ariaModifier = {
+    name: 'ariaDescribedBy',
     enabled: true,
-    order: 900,
-    fn: data => {
-      const { popper } = data.instance;
-      const target = getTarget();
+    phase: 'afterWrite',
+    effect: ({ state }) => {
+      return () => {
+        state.elements.reference.removeAttribute('aria-describedby');
+      };
+    },
+    fn: ({ state }) => {
+      const { popper, reference } = state.elements;
 
-      if (!show || !target) return data;
+      if (!show || !reference) return;
 
       const role = popper.getAttribute('role') || '';
       if (popper.id && role.toLowerCase() === 'tooltip') {
-        target.setAttribute('aria-describedby', popper.id);
+        reference.setAttribute('aria-describedby', popper.id);
       }
-      return data;
     },
   };
 
@@ -238,10 +242,7 @@ function OverlayTrigger({
         {...props}
         popperConfig={{
           ...popperConfig,
-          modifiers: {
-            ...popperConfig.modifiers,
-            ariaModifier,
-          },
+          modifiers: [ariaModifier].concat(popperConfig.modifiers || []),
         }}
         show={show}
         onHide={handleHide}

diff --git a/src/Popover.js b/src/Popover.js
@@ -45,9 +45,10 @@ const propTypes = {
   content: PropTypes.bool,
 
   /** @private */
-  scheduleUpdate: PropTypes.func,
+  popper: PropTypes.object,
+
   /** @private */
-  outOfBoundaries: PropTypes.bool,
+  show: PropTypes.bool,
 };
 
 const defaultProps = {
@@ -64,8 +65,8 @@ const Popover = React.forwardRef(
       children,
       content,
       arrowProps,
-      scheduleUpdate: _,
-      outOfBoundaries: _1,
+      popper: _,
+      show: _1,
       ...props
     },
     ref,

diff --git a/src/Tooltip.js b/src/Tooltip.js
@@ -55,10 +55,7 @@ const propTypes = {
   }),
 
   /** @private */
-  scheduleUpdate: PropTypes.func,
-
-  /** @private */
-  outOfBoundaries: PropTypes.any,
+  popper: PropTypes.object,
 
   /** @private */
   show: PropTypes.any,
@@ -77,8 +74,7 @@ const Tooltip = React.forwardRef(
       style,
       children,
       arrowProps,
-      scheduleUpdate: _,
-      outOfBoundaries: _1,
+      popper: _,
       show: _2,
       ...props
     },

diff --git a/test/OverlayTriggerSpec.js b/test/OverlayTriggerSpec.js
@@ -220,14 +220,18 @@ describe('<OverlayTrigger>', () => {
       },
     ].forEach(testCase => {
       describe(testCase.name, () => {
-        it('Should handle trigger without warnings', () => {
+        it('Should handle trigger without warnings', done => {
           mount(
             <OverlayTrigger trigger="click" overlay={testCase.overlay}>
               <button type="button">button</button>
             </OverlayTrigger>,
           )
             .find('button')
             .simulate('click');
+
+          // The use of Popper means that errors above will show up
+          //  asynchronously.
+          setTimeout(done, 10);
         });
       });
     });

diff --git a/types/components/Overlay.d.ts b/types/components/Overlay.d.ts
@@ -20,7 +20,24 @@ export type Placement =
   | 'left'
   | 'left-start';
 
+export interface OverlayInjectedProps {
+  show: boolean;
+  arrowProps: Record<string, any>;
+  popper: {
+    state: any;
+    outOfBoundaries: boolean;
+    placement: Placement;
+    scheduleUpdate: () => void;
+  };
+  [prop: string]: any;
+}
+
+export type OverlayChildren =
+  | React.ReactElement<OverlayInjectedProps>
+  | ((injected: OverlayInjectedProps) => React.ReactNode);
+
 export interface OverlayProps extends TransitionCallbacks {
+  children: OverlayChildren;
   container?: ComponentOrElement | ((props: object) => ComponentOrElement);
   target?: ComponentOrElement | ((props: object) => ComponentOrElement);
   show?: boolean;

diff --git a/types/components/OverlayTrigger.d.ts b/types/components/OverlayTrigger.d.ts
@@ -1,17 +1,17 @@
 import * as React from 'react';
 
-import Overlay from './Overlay';
+import { OverlayChildren, OverlayProps } from './Overlay';
 
 type TriggerType = 'hover' | 'click' | 'focus';
 
-export interface OverlayTriggerProps
-  extends React.ComponentPropsWithRef<typeof Overlay> {
+export interface OverlayTriggerProps extends Omit<OverlayProps, 'children'> {
   children: React.ReactNode;
   trigger?: TriggerType | TriggerType[];
   delay?: number | { show: number; hide: number };
   defaultShow?: boolean;
   flip?: boolean;
-  overlay: React.ReactNode | (() => React.ReactNode);
+  overlay: OverlayChildren;
+
   target?: never;
   onHide?: never;
   show?: never;

diff --git a/types/index.d.ts b/types/index.d.ts
@@ -20,6 +20,6 @@
 //                 Duong Tran <https://github.com/t49tran>
 //                 Bartosz Łaniewski <https://github.com/Bartozzz>
 // Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped
-// TypeScript Version: 3.1
+// Minimum TypeScript Version: 3.5
 
 export * from './components';
diff --git a/www/package.json b/www/package.json
@@ -30,7 +30,7 @@
     "gatsby-plugin-sass": "^2.1.30",
     "gatsby-remark-prismjs": "^3.3.35",
     "gatsby-source-filesystem": "2.1.55",
-    "gatsby-transformer-react-docgen": "^4.1.3",
+    "gatsby-transformer-react-docgen": "^5.0.29",
     "gatsby-transformer-remark": "^2.6.58",
     "gh-pages": "^2.2.0",
     "holderjs": "^2.9.6",
@@ -42,7 +42,7 @@
     "prismjs": "^1.19.0",
     "prop-types": "^15.7.2",
     "react": "^16.13.0",
-    "react-docgen": "^4.1.1",
+    "react-docgen": "^5.3.0",
     "react-dom": "^16.13.0",
     "react-live": "^2.2.2",
     "remark-slug": "^5.1.2",
@@ -71,5 +71,8 @@
     "deploy": "gatsby build --prefix-links && yarn push-docs",
     "push-docs": "gh-pages -d public --branch master --repo \"https://github.com/react-bootstrap/react-bootstrap.github.io.git\"",
     "sort": "import-sort 'src/pages/**/*.js'"
+  },
+  "dependencies": {
+    "ast-types": "^0.13.2"
   }
 }
diff --git a/www/resolveHocComponents.js b/www/resolveHocComponents.js
@@ -1,15 +1,12 @@
 const { camelCase, upperFirst } = require('lodash');
 const { resolver, utils } = require('react-docgen');
+const { namedTypes: types, visit } = require('ast-types');
 const buildParser = require('react-docgen/dist/babelParser').default;
 
 const parser = buildParser();
 
-module.exports = (ast, recast) => {
-  const {
-    types: { namedTypes: types },
-  } = recast;
-
-  let components = resolver.findAllComponentDefinitions(ast, recast);
+module.exports = ast => {
+  let components = resolver.findAllComponentDefinitions(ast);
 
   const getComment = path => {
     let searchPath = path;
@@ -26,7 +23,7 @@ module.exports = (ast, recast) => {
     return comment;
   };
 
-  recast.visit(ast, {
+  visit(ast, {
     visitCallExpression(path) {
       if (types.ExpressionStatement.check(path.node)) {
         path = path.get('expression');
@@ -52,15 +49,14 @@ module.exports = (ast, recast) => {
             : property.value.raw;
       }
 
-      let comp = recast.parse(
-        `
+      const src = `
 import React from 'react';
 import PropTypes from 'prop-types';
 
 ${comment || ''}
 export default class ${upperFirst(
-          camelCase(prefixNode.value),
-        )} extends React.Component {
+        camelCase(prefixNode.value),
+      )} extends React.Component {
   static propTypes = {
     /** @default ${prefixNode.raw} */
     bsPrefix: PropTypes.string.isRequired,
@@ -73,11 +69,12 @@ export default class ${upperFirst(
     return null
   }
 }
-        `,
-        { esprima: parser },
-      );
+        `;
+
+      let comp = parser.parse(src);
+      comp.__src = src;
       components = components.concat(
-        resolver.findExportedComponentDefinition(comp.program, recast),
+        resolver.findExportedComponentDefinition(comp),
       );
       return false;
     },

diff --git a/www/src/components/NavMain.js b/www/src/components/NavMain.js
@@ -95,15 +95,16 @@ function NavMain({ activePage }) {
       </Nav>
       <Nav className="ml-auto pr-md-5">
         <StyledDropdown id="t-version">
-          <Dropdown.Toggle id="dropdown-version" variant="outline-light">
-            v{config.version} (Bootstrap{' '}
+          <Dropdown.Toggle id="dropdown-version" as={StyledNavLink}>
+            v{config.version} (
+            <span className="d-none d-lg-inline">Bootstrap </span>
             {config.bootstrapVersion
               .split('.')
               .slice(0, 2)
               .join('.')}
             )
           </Dropdown.Toggle>
-          <Dropdown.Menu className="w-100">
+          <Dropdown.Menu className="w-100" role="menu">
             <Dropdown.Item href="https://react-bootstrap-v3.netlify.com">
               v0.32.4 (Bootstrap 3)
             </Dropdown.Item>

diff --git a/www/src/examples/Overlays/OverlayTrigger.js b/www/src/examples/Overlays/OverlayTrigger.js
@@ -1,5 +1,9 @@
 function renderTooltip(props) {
-  return <Tooltip {...props}>Simple tooltip</Tooltip>;
+  return (
+    <Tooltip id="button-tooltip" {...props}>
+      Simple tooltip
+    </Tooltip>
+  );
 }
 
 const Example = () => (

diff --git a/www/src/examples/Overlays/ScheduleUpdate.js b/www/src/examples/Overlays/ScheduleUpdate.js
@@ -1,11 +1,12 @@
 const UpdatingPopover = React.forwardRef(
-  ({ scheduleUpdate, children, ...props }, ref) => {
+  ({ popper, children, show: _, ...props }, ref) => {
     useEffect(() => {
       console.log('updating!');
-      scheduleUpdate();
-    }, [children, scheduleUpdate]);
+      popper.scheduleUpdate();
+    }, [children, popper]);
+
     return (
-      <Popover ref={ref} {...props}>
+      <Popover ref={ref} content {...props}>
         {children}
       </Popover>
     );