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

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

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

HeVictor
Copy link
Contributor

…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
Copy link
Contributor

LukasTy commented Mar 15, 2023

@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 HeVictor force-pushed the add-tslib-as-deps branch from 5db93c2 to 883efe6 Compare March 16, 2023 12:40
@HeVictor
Copy link
Contributor Author

@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
Copy link

oliviertassinari commented Mar 18, 2023

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: mui/mui-x#832.

@HeVictor
Copy link
Contributor Author

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?

@dmtrKovalenko dmtrKovalenko merged commit 0319900 into dmtrKovalenko:master Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing tslib dependency?
4 participants