React-popper: Experiment a new hooks based API

[FezVrasta]

I'd like to add, on top of the existing render-props based API, a way to use react-popper with the new React hooks.

I can think of a possible API to look like the following:

import { usePopper } from 'react-popper';

const Example = () => {
  const [reference, popper, arrow] = usePopper(options);

  return (
    <Fragment>
      <button type="button" ref={reference.ref}>
        Reference element
      </button>

      <div ref={popper.ref} style={popper.style} data-placement={popper.placement}>  
        Popper element
        <div ref={arrow.ref} style={arrow.style} />
      </div>
    </Fragment>
  );
}

I still have to think of a way to allow multiple poppers handled by the same reference, I think a possibility would be to expose an utility that merges reference.ref into a single one:

import { usePopper, mergeRef } from 'react-popper';

const Example = () => {
  const [reference1, popper1, arrow1] = usePopper(options);
  const [reference2, popper2, arrow2] = usePopper(options);

  return (
    <Fragment>
      <button type="button" ref={mergeRef(reference1.ref, reference2.ref)}>
        Reference element
      </button>

      <div ref={popper1.ref} style={popper1.style} data-placement={popper1.placement}>  
        Popper element
        <div ref={arrow1.ref} style={arrow1.style} />
      </div>

      <div ref={popper2.ref} style={popper2.style} data-placement={popper2.placement}>  
        Popper element
        <div ref={arrow2.ref} style={arrow2.style} />
      </div>
    </Fragment>
  );
}

What do you think? Suggestions? Ideas?

[SidKH]

Looks awesome!

Merge ref utility sounds like a great solution. Should be easy to implement as well

const mergeRef = (...refs) => (ref) => refs.forEach(fn => fn(ref))

Also in multiple poppers example a thing to keep in mind is that change in a state of first usePopper will trigger re-render of component and therefore calls second usePopper and vise versa but since react-popper already has checks to prevent unnecessary re-computations it should be fine.

[TrySound]

We may omit mergeRef utility with just changing refs from return values to arguments

const Example = () => {
  const referenceRef = useRef(null)
  const popperRef = useRef(null)
  const arrowRef  = useRef(null)
  const [popper, arrow] = usePopper(referenceRef, popperRef, arrowRef, options);

  const { width, height } = useMeasure(referenceRef)

  return (
    <Fragment>
      <button type="button" ref={referenceRef}>
        Reference element
      </button>

      <div ref={popperRef} style={popper.style} data-placement={popper.placement}>  
        Popper element
        <div ref={arrowRef} style={arrow.style} />
      </div>
    </Fragment>
  );
}

This way referenced may be passed even from props without necessity to wrap reference with popup component.
https://github.com/Andarist/use-onclickoutside

[FezVrasta]

@TrySound that sounds like a great idea, it should even further simplify the code.

[nassimbenkirane]

It would be great to also be able to merge the refs with external refs,
and/or to give only some refs as params:

const popperRef=useRef();
const {popper, arrow, managerRef, arrowRef} = usePopper({popperRef});

[FezVrasta]

@nassimbenkirane if we are going with @TrySound suggestion I don't think we need it.

https://github.com/FezVrasta/react-popper/issues/241#issuecomment-439604034

[nassimbenkirane]

@FezVrasta, @TrySound,
You're right that if with this suggestion, we don't need to merge refs,
I think it's a good solution, I just think that providing a custom ref should be an 'opt in' behaviour, as it would make the default API a little more verbose.

I'd like to be able to provide only a subset of the refs and take the rest from the objects provided by usePopper

const Example = () => {
  const referenceRef = useRef(null)
  const {reference, popper, arrow} = usePopper({referenceRef}, options);

  const { width, height } = useMeasure(referenceRef)

  return (
    <Fragment>
      <button type="button" ref={reference.ref}> // or referenceRef as it would point to the same object
        Reference element
      </button>

      <div ref={popper.ref} style={popper.style} data-placement={popper.placement}>  
        Popper element
        <div ref={arrow.ref} style={arrow.style} />
      </div>
    </Fragment>
  );
}

Here we only need a custom ref for the referenceRef

[TrySound]

@nassimbenkirane Such options usually makes api more complex. There should be only one way to do things in good api.

[TrySound]

Creating refs is not so hard with hooks. And if it will be widespread it won't be confusing.

[nassimbenkirane]

Hi @TrySound,

I kindly disagree with you on this one 🙂,
Good APIs should be simple to use for the general use case, and easy to extend for the 1-5% cases where you need more control.

In the general use case where you want one popper and don't need to tweek with the refs yourself (as in your example with useMeasure), compare this :

  const referenceRef = useRef(null)
  const popperRef = useRef(null)
  const arrowRef  = useRef(null)
  const [popper, arrow] = usePopper(referenceRef, popperRef, arrowRef, options);

to this :

const [reference, popper, arrow] = usePopper(options);

In the first case, one could say usePopper leaks implementation details, and it makes it more error-prone since there is more reponsability put on the developer. The second one is just easier.

I agree it's a tradeoff though

Also I'm not found of positional arguments for usePopper, as it would be a breaking change to add one additional argument in the future

That said, being able to provide the refs that we want to usePopper is a very good idea, and whatever the final API, I'm so excited for the hooks version !!
In our use case it will yield a massive simplification of our components 😁

[TrySound]

I don't insist on positional arguments. It's just an example. My point only against two sources of refs.

const referenceRef = useRef(null)
const popperRef = useRef(null)
const [reference, popper, arrow] = usePopper({
  referenceRef,
  popperRef,
})
// reference.ref === undefined
// popper.ref === undefined

[nassimbenkirane]

I get your point

[FezVrasta]

Remember that react-popper is meant to be a thin layer between React and Popper.js, it's not meant to be used directly, you will likely use something more user-friendly such as a tooltip library that relies on react-popper for the positioning.

So I'd prefer to keep the API verbose but powerful rather than easy to use but with gotchas to make it work in more advanced cases.

[nassimbenkirane]

@FezVrasta makes sense :)

[t3kkitz]

Instead of:
const [reference, popper, arrow] = usePopper(options);
I think better to use object destructuring coz remembering arguments order is 'meh':
const {reference, popper, arrow} = usePopper(options);
or you can use it with multiple instances with access by key
const popup1 = usePopper(options);
const popup2 = usePopper(options);
...
<div ref={popup1.ref}>elem<div>

[FezVrasta]

I don't know how much supported this would be, but we could have an iterable object to support both cases.. lol

const result = {
  reference: 'reference',
  popper: 'popper',
  arrow: 'arrow',
  [Symbol.iterator]() {
    let index = 0;
    let properties = Object.keys(this);
    let done = false;
    return {
      next: () => {
        done = index >= properties.length;
        let obj = {
          done,
          value: this[properties[index]],
          key: properties[index]
        };
        index++;
        return obj;
      }
    };
  }
};

[donysukardi]

I think it's better to let users pass in their own refs instead of letting the library create for them. This pattern promotes better composability.

As for the return value from usePopper, i'd vote for using object whenever the return value has more than two entries.

I took a stab at writing react-popper as a hook too - https://codesandbox.io/s/0xrj13y4n

[FezVrasta]

@donysukardi that's some huge code reduction 😱

Would you mind sending a PR which replaces the current code with this one? I'm going to create a v2 branch where we'll keep this code while we work on it and wait for the official release of hooks.

[donysukardi]

Sure thing! Let me try to add the typings and do that 👍

[denisborovikov]

I took a stab at writing react-popper as a hook too - https://codesandbox.io/s/0xrj13y4n

Seems it's required to compare incoming data in callbackFn with the previous one and update state only if the data changes. Otherwise, the whole component updates due to numerous callbackFn calls (and state updates) on scroll. Not sure if useCallback works here.

[FezVrasta]

@donysukardi are you still interested in that PR or should I do it?

[oadrian2]

Any updates on this?

[FezVrasta]

I'm gonna work on it as soon I get back to Europe I guess

[sgenoud]

I played with Popper.js in hooks a little bit. They make the integration so much simpler there might not even need for a library anymore. I wrote a little thing about that https://carrots.sgenoud.com/use-popper.js-in-react/

Feel free to start from my code if you want to have hooks as part of react-popper.

[k15a]

You will have to use callback refs instead of useRef otherwise you will have the same problem react-dnd had with their experimental hooks API. You can read more about that here: https://reactjs.org/docs/hooks-faq.html#how-can-i-measure-a-dom-node

[FezVrasta]

I'm working on this, I was converting the demo page and the tests, I'm not actually sure if the Hooks API helps in the react-popper case... Stuff got way more complex IMHO. 🤷‍♂️

[therealparmesh]

Check out:

https://www.npmjs.com/package/use-popper
https://github.com/sandiiarov/use-popper

This is what the company I'm at has been using for awhile. Maybe it can help with these discussions.

[denisborovikov]

As discussed above, declaring refs inside rather than accepting them as arguments don't look like a good idea.

[k15a]

Accepting them as arguments will lead to bugs so it's the only option.

[FezVrasta]

I was thinking that maybe a better approach could be:

const [Popper, Reference, Arrow] = usePopper();

And then keep the render-prop API for the 3 components returned by the hook. Doing so we get rid of the annoying Manager wrapper but we keep the ability to easily spawn multiple poppers from a single usePopper call.

[therealparmesh]

@denisborovikov Hmm, from what I can infer here it seems like people are still split on this.

[denisborovikov]

@therealparmesh It's not clear for me then, how to re-use the same DOM node.

@k15a check the updated code in the message above . It uses useState instead of useRef but still accepts ref (state) as an argument. I believe it was updated after your comment.

[therealparmesh]

@denisborovikov How do you feel about something like https://github.com/FezVrasta/react-popper/issues/241#issuecomment-439604034?

https://codesandbox.io/s/0qjk4w5v0l

[benbayard]

I just wanted to throw my opinion in that the benefits of positional arguments are numerous:

1. easier to support multiple poppers (because you don't have to rename the keys)
2. Follows the patterns in existing hooks like useState
3. needing to know the return order is alleviated with sufficient static typing. Editors that support static types won't need to worry about this at all.

[FezVrasta]

With the upcoming Popper 2, an hypothetical hook would look like this:

import { useLayoutEffect, useRef } from 'react';
import { createPopper } from '@popperjs/core/lib/popper.js';

export const usePopper = (options = {}) => {
  const referenceRef = useRef();
  const popperRef = useRef();

  const popperInstanceRef = useRef();

  useLayoutEffect(() => {
    const popperInstance = createPopper(
      referenceRef.current,
      popperRef.current,
      options
    );

    popperInstanceRef.current = popperInstance;

    return () => {
      popperInstance.destroy();
    };
  }, []);

  useLayoutEffect(() => {
    popperInstanceRef.current.setOptions(options);
    popperInstanceRef.current.update();
  }, [options]);

  return {
    reference: referenceRef,
    popper: popperRef,
  };
};

[therealparmesh]

@FezVrasta This looks great! One minor detail - Could we also expose scheduleUpdate from the hook?

[FezVrasta]

yes of course, this is just a quick draft to show the potential of the new Popper API.

I'd also need to expose the arrowRef somehow.

[FezVrasta]

import { useLayoutEffect, useRef } from 'react';
import { createPopper } from '@popperjs/core/lib/popper.js';


export const usePopper = (options = {}) => {
  const referenceRef = useRef();
  const popperRef = useRef();
  const arrowRef = useRef();
  const popperInstanceRef = useRef();

  const buildOptions = options => ({
   ...options,
    modifiers: [
      ...options.modifiers,
      { name: 'arrow', options: { element: arrowRef.current } },
    ]
  });

  useLayoutEffect(() => {
    const popperInstance = createPopper(
      referenceRef.current,
      popperRef.current,
      buildOptions(options)
    );

    popperInstanceRef.current = popperInstance;

    return () => {
      popperInstance.destroy();
    };
  }, []);

  useLayoutEffect(() => {
    popperInstanceRef.current.setOptions(buildOptions(options));
    popperInstanceRef.current.update();
  }, [options]);

  return {
    reference: referenceRef,
    popper: popperRef,
    arrow: arrowRef,
    update: popperInstanceRef.current.update,
  };
};

Probably it will end up looking like this.

edit: anyways, you can try this out today! Just install @popperjs/core@alpha

[atomiks]

Here's a CodeSandbox demo of the hook for rc.2

There are two examples:

• Default: import from the default path, @popperjs/core, to include all the modifiers available (even the ones you don't use)
• Lite: import from the lite path, @popperjs/core/lib/popper-lite, to import only the most necessary modifiers, and add modifiers as you go. See tree-shaking in the docs for more info.

There's also the base path, @popperjs/core/lib/popper-base, which includes no modifiers at all, but I haven't put that in the demo. The usage is the same as lite, it's just missing 4 core modifiers.

The hooks get repeated for each path, and maybe they will be in separate files to import from, i.e.

• Default hook: import {usePopper} from '@popperjs/react';
• Lite hook: import {usePopper} from '@popperjs/react/lite';

unless there is a better way

[FezVrasta]

I was considering adding an option to pass a custom createPopper function, so that consumers can override the default one with one configured with their needs.

[atomiks]

That does make sense, dependency-injectiony type of thing. How can there be a default one though? I don't think we can import from anywhere since it will add unnecessary code if they don't use that import path. It seems like it would be required to inject a createPopper function where the user chooses the path.

[FezVrasta]

I suppose tree-shaking should be able to strip out the default createPopper if it ends up unused, but I need to verify it.

[TrySound]

@FezVrasta Treeshaking will work if you will add annotation

const createPopper = /*#__PURE__*/popupGenerator()

[atomiks]

Example of injection technique - where would the annotation go @TrySound?

[FezVrasta]

Is /*#__PURE__*/ necessary if the library defines sideEffects: false in its package.json?

edit: it looks like it's necessary, I added a Babel plugin to automatically add those annotations, it's on master.

[FezVrasta]

I'm thinking about something like this...
https://codesandbox.io/s/recursing-northcutt-porqh

  const referenceRef = useRef();
  const popperRef = useRef();
  const arrowRef = useRef();

  const {
    styles: { popper: popperStyle, arrow: arrowStyle },
    attributes: { popper: popperAttributes }
  } = usePopper(referenceRef, popperRef, {
    modifiers: [
      { name: "offset", options: { offset: [0, 8] } },
      { name: "arrow", options: { element: arrowRef.current } }
    ]
  });

so that we keep the same API of Popper 2

[renke]

A few API suggestions (based on my own hook version of popper.js):

// optional, true by default, enables or disables popper.js entirely
const enabled = true;

// optional, [] by default, updates popper based on other props being changed
const dependencies = [someDataThatChangesPopperPosition]

// optional, partial, default options should be exported by popper though
const popperOptions = {
// ...
}

// order of parameters could be changed or "named" parameters could be used instead, of course
const {popperProps, arrowProps, popperInstance} = usePropper(referenceRef, popperRef, enabled, dependencies, popperOptions)

In addition to that, it might make sense to expose a low-level hook (default config, no popper/arrow props) and high-level hook (which uses the low-level one and behaves like one above/your latest example).

[jkillian]

Great discussions! I was curious if there's a WIP branch or anything demonstrating the new API? I'm considering switching my codebase over from Tether -> Popper, I'd just rather wait to do it until the hooks API is released so that we don't have to make another transition later.

[jquense]

if anyone is looking for some public implementation react-overlays has a popper hook: https://github.com/react-bootstrap/react-overlays/blob/master/src/usePopper.js

[designbyadrian]

This looks much more convoluted than the Manager/Reference/Popper structure of 1.2 – what is there to gain?

[ignatevdev]

@jquense I was trying to make my own hook the same way you did, but I got really confused if I should try to prevent unnecessary state changes.

Modifiers are run on every scroll event, which would trigger dozens of setState calls. In theory, this can result in a performance issue if someone tries to put an unoptimized component inside the dropdown, and I'm not sure if I should try to debounce it in my component.

Maybe someone has already thought about it, but it seems like every popper adapter for react just doesn't care about it.

Great hook otherwise, I might be stealing some code from it 🤓

[jquense]

I don't think you need to worry about it unless you are running into perf issues. Popper already schedules and dedupes the events via requestAnimationFrame so it should be smooth even if it's noisy. If it does causes issues you can disable updates from events, which is partly why its a first class option on the hook

[ignatevdev]

Alright, thanks!

I have also tested your hook implementation and would like to share a few suggestions, because they might be relevant to anyone who tries to do the same and also relevant to the issue.

1. Passing elements instead of refs might cause issues on the initial mount of the component

In my case, because refs are created before the component mounts, the initial useEffect call was receiving null for both reference and popper elements. And because when ref is saved its value is mutated, it doesn't really matter if you put them as a dependency to useEffect, it most likely won't be triggered. However, if you switch arguments to refs and get the element instances inside the hook - the initial mount would receive the elements.

1. The first approach fixes the initial mount problem, but it doesn't account for the ref updates. If for some reason react recreates the popper element during the component lifecycle, the initial ref value would point to an already unmounted element and useEffect will not detect the change. This will cause popper to behave incorrectly and can't be left as is.

To fix that, the best way is to accept only the reference element ref as an argument and create the popper ref inside the hook as a function.

This is already done in the current version of react-popper, but inside the class component.
For me it looks something like this:

const saveRef = (el: HTMLElement) => {
    if (!el || el === popperElementRef.current) return;

    popperElementRef.current = el;

    destroyPopper();

    initPopper();
};

And it seems to me that this is the only way to actually support the full lifecycle of the popper component and react to the ref changes and every hook implementation should take this into account.

[BenGladman]

I have just implemented a hooks api closely based on the one in react-overlays mentioned above. I came across the problems mentioned by @ignatevdev . To solve, instead of using useRef to hold the elements, I use useState, so the component is rerendered when the dom element changes. Like this:

  const [referenceEl, setReferenceEl] = useState<HTMLElement | null>(null);
  const [contentEl, setContentEl] = useState<HTMLElement | null>(null);

  const { styles, arrowStyles, placement } = usePopper(
    referenceEl,
    contentEl,
    options
  );

  return (
    <>
      <Reference ref={setReferenceEl} />
      {show &&
        createPortal(
          <Content
            ref={setContentEl}
            style={styles}
            placement={placement}
          />,
          document.body
        )}
    </>
  );

...but I'm wondering, is it bad to setState during render? It seems to work okay...

[Velenir]

I would like to add another usePopper example implementation
https://codesandbox.io/s/react-popper-hook-2z49d
Two variants: DumbExample -- where popper takes care of styling, and SmartExample -- react handles styling.

I hope it will be of use for someone.

[ignatevdev]

@BenGladman

I have also tried that approach, actually, but in my case it was causing infinite loop, which is why I ended up using refs. Maybe there is something wrong with how I render the actual popper, but I didn't dive too much into it because I was migrating the working code from react-popper and wanted to achieve the same behaviour. But if it works for you without a problem, I guess it is an option.

[jquense]

To clarify folks, the RO hook does not accept refs intentionally because useRef is the wrong thing to use when you need to be updated when they change. Instead you should use a "callback ref" (react's confusing name not mine).

const [el, setEl] = useState<HTMLElement>(null)
const popper = usePopper(el)

return <div ref={setEl} />

As @ignatevdev noted the problem with refs is you cannot respond to changes very easily. instead, useState is the more correct thing to do here if you need to respond to element changes like we do. That it is null on the first pass is unimportant in practice (in our experience here, and in react-bootstrap. b/c the element is set and updated before any DOM rendering issues could happen.

@BenGladman it's fine to call set state in render: official docs. If anyone is running into a loop, that's because your updates aren't guarded. You need to avoid setting on _every_ render b/c a setState will trigger another render. Same as how it's fine to set state in componentDidUpdate and with the same caveats for loops.

references:
https://reactjs.org/docs/refs-and-the-dom.html#callback-refs
https://twitter.com/sebmarkbage/status/1091808948523581440

[BenGladman]

it's fine to call set state in render

Nice, thanks for the docs link @jquense , that has put my mind at ease!

[FezVrasta]

Here's a PR with a first draft of the usePopper Hook: https://github.com/popperjs/react-popper/pull/343

I'd also like to rewrite the existing render-props components to use the new Hook internally, but I'll wait for some feedback on the newly proposed Hook before I proceed.