Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Popper changed home #19358

Closed
RDIL opened this issue Jan 23, 2020 · 17 comments · Fixed by #21761
Closed

Popper changed home #19358

RDIL opened this issue Jan 23, 2020 · 17 comments · Fixed by #21761
Labels
external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request
Milestone

Comments

@RDIL
Copy link
Contributor

RDIL commented Jan 23, 2020

I'm getting this when trying to update MUI:

warning @material-ui/core > popper.js@1.16.1: Popper changed home, find its new releases at @popperjs/core
@eps1lon
Copy link
Member

eps1lon commented Jan 23, 2020

Will sync with popper.js on this one. A package shouldn't issue warnings just because it got a new release.

@eps1lon
Copy link
Member

eps1lon commented Jan 23, 2020

On a second thought: I guess the deprecation warning is fine. It says that new releases are found elsewhere not that something might be broken. We'll check out if we can transition safely to v2.

As long as we don't receive bug reports using popper 1.x this warning is harmless.

@eps1lon eps1lon added new feature New feature or request external dependency Blocked by external dependency, we can’t do anything about it labels Jan 23, 2020
@oliviertassinari
Copy link
Member

I have seen somewhere that v2 drops IE 11 support.

@RDIL
Copy link
Contributor Author

RDIL commented Jan 23, 2020

What is popper used for again?

@FezVrasta
Copy link

FezVrasta commented Jan 23, 2020

I have seen somewhere that v2 drops IE 11 support.

It should work, but it requires some polyfills that are not included.

What is popper used for again?

It's used to position tooltips, popovers, dropdowns, etc.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 27, 2020

@FezVrasta Thanks for the details! IE 11 usage is still incredibly high in enterprises, a market our library resonates well with, e.g #14420 (comment).

The more I think about it, the more replicating React strategy with browser support could be interesting.
In any case, requiring polyfills would be a breaking change, it prevents us from upgrading before the next major window: v5 (Q3-Q4 2020), exactly like it impacts Bootstrap.

Regarding the warning on npm, I'm wondering, wouldn't it be enough to focus on notifying the few dependencies upstream about the existence of this new version? By order of usage, I imagine: Bootstrap, Material-UI, and react-popper. Do we have more?


Popper.js was first introduced a few years because Bootstrap was using it. However, I have never really benchmarked to seek the best option available for our needs. It could be a nice opportunity to do such for v5. I have started a quick benchmark, I could be great to continue it with the React libraries we usually benchmark with.

At the minimum, it would be great to identify the set of features we really need to see which one we can drop and potentially save bundle size.

@oliviertassinari oliviertassinari added this to the v5 milestone Jan 27, 2020
@eps1lon
Copy link
Member

eps1lon commented Jan 27, 2020

It should work, but it requires some polyfills that are not included.

Array.prototype.find,Array.prototype.includes,String.prototype.includes,Array.from,Object.entries,Promise,Object.assign

-- https://popper.js.org/docs/v2/browser-support/#ie11

That's a little bit too much. Especially Promise might be too big. Since v1 is working fine for us we probably want to stay on that version.

@FezVrasta
Copy link

FezVrasta commented Jan 27, 2020

Array.from is not required, I need to fix the docs.

I think I can get rid of Array.prototype.includes also.

That should reduce the polyfills size to 5.2kB.

Popper v1 was 7.3kb, Popper v2 is 5.23kB + 5.2kB of polyfills (that can be reused by other dependencies, code, etc. ) = 10.43kB

If you just need Popper v2 lite, it’s gonna be 2.81 kB + 5.2kB = 8.01kB

—-

Regarding the Popper alternatives, I’m not aware of any library with the same robustness and more lightweight.

@atomiks
Copy link
Contributor

atomiks commented Jan 28, 2020

Especially Promise might be too big

Who is not polyfilling (if they need to) Promise in their apps already? All data fetching-based code is using that. Same could be said for most of those other polyfills too in most areas of apps. They're pretty essential methods.

Regardless, the polyfill.io service is great because it means only IE11 gets the bloat and evergreen browsers get the lightest version. Bundling in polyfills prevents this benefit. The vast majority using IE11 is also likely on a fast corporate internet connection so it's even less of a problem when using this technique.

@FezVrasta
Copy link

Regarding the warning on npm, I'm wondering, wouldn't it be enough to focus on notifying the few dependencies upstream about the existence of this new version? By order of usage, I imagine: Bootstrap, Material-UI, and react-popper. Do we have more?

There are more than half million projects (only on GitHub) with Popper as dependency, not all of them use Bootstrap or MUI, we want to notify as many users as possible about our new release, especially because it fixes a lot of long standing bugs that affect billion of users every day.

@eps1lon
Copy link
Member

eps1lon commented Jan 28, 2020

We don't want to break peoples code as all software should do. We didn't require these polyfills before and that's pretty much the end of this discussion.

We don't have the time to upgrade a dependency that is working fine so far and would introduce breaking changes just to get rid of a deprecation warning.

Just like yarn v1, popper v1 didn't stop working just because v2 got released. No need to panic upgrade.

@FezVrasta
Copy link

I completely agree, you are free to use v1 for as long as you need. The warning is there because of lack of a better system to notify users about a package name change. You can ignore it

@oliviertassinari
Copy link
Member

oliviertassinari commented Jan 29, 2020

@FezVrasta It's pretty cool to see that Popper v2 is -12% smaller than Popper v1, (assuming we can trust bundlephobia relative values). Congrats :).

Regarding the Popper alternatives, I’m not aware of any library with the same robustness and more lightweight.

It's very possible, at some point, we (as developers) can't do much about the bundle size/feature density, it's set by external constraints :). I think that we can focus on identifying the minimum set of features we really need and remove everything that we don't. The plugin system allows removing unneeded features which is a great design decision.

There are more than half million projects (only on GitHub) with Popper as dependency

I was wondering about the direct dependencies, users can't have a direct influence on transitive ones (they can have an indirect one by using different packages ). The download stats, assuming we can rely on them made me suggest that in a strong and large majority of the time, popper.js is a transitive dependency: https://npm-stat.com/charts.html?package=popper.js&package=bootstrap&package=react-popper.
However, I can understand that it can be frustrating not to be a direct dependency, that it's hard to raise awareness of the careful work done behind the scene. I respect this. It made me think of corejs <=> babel link.

@atomiks
Copy link
Contributor

atomiks commented Jan 29, 2020

@oliviertassinari Popper 2 is actually about 23% smaller when you include every modifier. (The Bundlephobia size seems wrong –– the umd is only 5.8 KB minzipped).

If you include only the essential modifiers on top of Lite, it's around 30% smaller. If you only use Lite then it's 60% smaller but that's not practical in real usage.

@re-thc
Copy link

re-thc commented Feb 9, 2020

Is IE11 usage in enterprises actually a thing as of today? The link quoted was in Oct 2019. It's been a few months. The main deciding factor is Windows 10 adoption since Edge becomes the default. Over 2019 I've seen lots of enterprises move to Windows 10. This will continue into 2020 and by the time v5 is out (Q3) I'm concerned that IE usage is likely to drop significantly and a lot of work is done for nothing.

@dandv
Copy link
Contributor

dandv commented May 29, 2020

This prompted me to file a feature request for npm to automatically show what package(s) depend on the deprecated one.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 29, 2020

@dandv Thanks for raising, something is off with #20433.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external dependency Blocked by external dependency, we can’t do anything about it new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants