Skip to content

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

Merged
dmtrKovalenko merged 1 commit into
dmtrKovalenko:masterfrom
HeVictor:add-tslib-as-deps
Mar 12, 2024
Merged

fix: adds tslib deps to fix the error of it not being found when impo…#643
dmtrKovalenko merged 1 commit into
dmtrKovalenko:masterfrom
HeVictor:add-tslib-as-deps

Conversation

@HeVictor

@HeVictor HeVictor commented Mar 14, 2023

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

LukasTy commented Mar 15, 2023

Copy link
Copy Markdown
Contributor

@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

HeVictor commented Mar 16, 2023

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

@HeVictor HeVictor mentioned this pull request Mar 16, 2023
Closed
@oliviertassinari

oliviertassinari commented Mar 18, 2023 •
edited
Loading

Copy link
Copy Markdown

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

HeVictor commented Mar 19, 2023

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing tslib dependency?

4 participants

@HeVictor @LukasTy @oliviertassinari @dmtrKovalenko