search

floating-ui / react-popper

🍿⚛Official React library to use Popper, the positioning library
https://popper.js.org/react-popper/
MIT License
2.52k stars 225 forks source link

Using only hooks from usePopper causes issues with rollup #379

Open oageyeva opened 4 years ago

oageyeva commented 4 years ago

Reproduction demo

I am using usePopper hook for auto positioning a dropdown. The code works fine in a sandbox and when running in the development environment. However, it does not pass the build pipeline with rollup.

I figure out that the problem is that react-popper package has both components and hooks and when only hooks are being pulled rollup thinks that createElement used for components is being imported, but not being used. If I import Manager from react-prop into one of my modules even though the component is not being used that fixes the pipeline problem, but does not look like a right solution. Other packages that have both hooks and components like downshift are passing the pipeline Ok.

Is it possible that hooks and components are not being separated properly?

Sandbox code

Steps to reproduce the problem

  1. Create a component using usePopper hook, but not using any components.
  2. run npm run build (build: `rollup -c "--format" "umd" )

What is the expected behavior?

Expect build to pass.

What went wrong?

The build fails with an error. image

Packages versions

d3caf commented 4 years ago

I'm also having this issue it seems. However, importing Manager from react-popper doesn't seem to solve the issue for me. But we are using only hooks in our project and building via rollup -c and getting update is not a function. Same issue with trying to use forceUpdate()

Update: I think in my case this isn't a Rollup issue. When testing in Storybook (which uses webpack) I get the same issue. For me, it seems that the issue is that Popper isn't instantiating on mount unless something causes my component to re-render. So update() is set to null as seen in usePopper.js. I think this is a new issue with Popper v2 as I didn't have this issue in v1 using the components. I'm curious to know whether you're seeing the same behavior.

d3caf commented 4 years ago

Ok, so I think I've found the issue. I didn't look closely enough at the docs. I was surprised to find that instead of using useRef, the example in the docs uses useState to store the refs. It looks like in your sandbox you're using state as well, so I guess this isn't the same issue for you. However, I am also using rollup for my dist builds and am not seeing this issue.