search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
94.13k stars 32.35k forks source link

Popper changed home #19358

Closed RDIL closed 4 years ago

RDIL commented 4 years ago

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 commented 4 years ago

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

eps1lon commented 4 years ago

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.

oliviertassinari commented 4 years ago

I have seen somewhere that v2 drops IE 11 support.

RDIL commented 4 years ago

What is popper used for again?

FezVrasta commented 4 years ago

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 commented 4 years ago

@FezVrasta Thanks for the details! IE 11 usage is still incredibly high in enterprises, a market our library resonates well with, e.g https://github.com/mui-org/material-ui/issues/14420#issuecomment-536642248.

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.

eps1lon commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

oliviertassinari commented 4 years ago

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