dmtrKovalenko / date-io

Abstraction over common javascript date management libraries
MIT License
726 stars 90 forks source link

fix: adds tslib deps to fix the error of it not being found when impo… #643

Closed HeVictor closed 8 months ago

HeVictor commented 1 year ago

…rting date-io libs

Resolves #248

Just want to make sure that I should be adding this dependency to the root and not within the hijri package where the error has been occurring in this CodeSandbox demo?

On an additional note I share the sentiment from #641 that perhaps it might be better for some of the deps such as typescript and ts-jest to be moved into devDependencies eventually which means that tslib can also be moved into there as well. But hopefully this PR can fix the issue at hand for now

LukasTy commented 1 year ago

@HeVictor I could be wrong, but as far as I can see—this fix wouldn't help with the original issue. This package is already installed in the root repo as a sub-dependency. The issue is that the hijri and jalaali packages do not have it as an explicit dependency or a peer dependency.

HeVictor commented 1 year ago

@LukasTy Thanks for the tip, I have amended the PR to add it in the peerDependencies for the packages hijri and jalaali instead. I think this is a good solution for now but as you mentioned in #248, I do agree that in the long-term raising the target to ES6 would be ideal. Maybe down the road that could be the long-term option to go

oliviertassinari commented 1 year ago

I'm also concerned with having the need for tslib to be in the bundle in the first place, it duplicates with @babel/runtime, it would likely save bundle size, if we can kill it.

Screenshot 2023-03-18 at 21 31 21

https://npm.anvaka.com/#/view/2d/%2540mui%252Fx-date-pickers

I did this PR a while back: https://github.com/mui/mui-x/pull/832.

HeVictor commented 1 year ago

Do we think the best course of action is to just try and remove the tslib and other related dependencies out of the main deps then, and just skip this band-aid fix altogether?