mui-x icon indicating copy to clipboard operation
mui-x copied to clipboard

[pickers] Increase min moment peer dependency version

Open oliviertassinari opened this issue 2 years ago • 1 comments

Act on https://groups.google.com/a/mui.com/g/security/c/0fPj4R9qqEY.

For example https://security.snyk.io/package/npm/moment Screenshot 2023-02-25 at 18 21 42

oliviertassinari avatar Feb 25 '23 17:02 oliviertassinari

Messages
:book: Netlify deploy preview: https://deploy-preview-8046--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 623.6 1,017.3 627.7 785.88 150.744
Sort 100k rows ms 601 1,207 844.2 925.22 199.372
Select 100k rows ms 165.6 304.7 274.9 253.54 52.84
Deselect 100k rows ms 140.6 403.9 193.5 230.84 90.806

Generated by :no_entry_sign: dangerJS against 1b0faeec3578c8562c89c46b05e6a7a10804e968

mui-bot avatar Feb 25 '23 17:02 mui-bot

Is it really a breaking change if we are bumping the peer dependency version a few patches up? 🤔 I mean that if we do back-port it to master and release in v5, what kind of a bump would you suggest? Minor? 🤔

@LukasTy I haven't tried but I think that it could break installations because of https://github.blog/2021-02-02-npm-7-is-now-generally-available/#peer-dependencies

Screenshot 2023-02-27 at 16 49 27

https://stackoverflow.com/questions/66239691/what-does-npm-install-legacy-peer-deps-do-exactly-when-is-it-recommended-wh

So I imagine that on v5, either developers use the latest version and it won't change much for them, or they use an older version and it will break. No real preference, I didn't work on master because it wasn't worth it, relative to my role vs. the other problems I can spend time on solving.

oliviertassinari avatar Feb 27 '23 15:02 oliviertassinari

Yes, it could potentially be an issue, but that would require some interesting setup, where some package would have either a specific moment version or lower or equal (<=) to the one we need t bump to. I do understand, that technically this is a breaking change and releasing under a minor/patch bump would not be strictly semver. TL.DR.: Sure, we can avoid the change in v5. The potential risk might not be worth it, because end-users can update the used library version as they see fit. 👌

LukasTy avatar Feb 27 '23 16:02 LukasTy