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

Re-evaluate requirement for tslib as a dependency #47

Closed
oliviertassinari opened this issue Jul 7, 2020 · 6 comments · Fixed by #832
Closed

Re-evaluate requirement for tslib as a dependency #47

oliviertassinari opened this issue Jul 7, 2020 · 6 comments · Fixed by #832
Labels
core Infrastructure work going on behind the scenes
Milestone

Comments

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 7, 2020

This is a follow-up on a conversation we have started in #44 (comment). The tslib dependency was added in order to make codesandbox runs successfully. It's unclear if the requirement is coming from codesandbox itself or from a misconfiguration of the building pipeline.

It's also to be noted that we use babel-runtime on @material-ui/core which might duplicate in the bundle for our users.

@dmtrKovalenko Considering we have a similar building stack on the pickers, do you have any idea of what could be wrong? :)

I have added the "Post MVP" milestone as we can/should delay the resolution to later.

@oliviertassinari oliviertassinari added this to the Post MVP milestone Jul 7, 2020
@dmtrKovalenko
Copy link
Member

Yes, in pickers we followed up on the usage of Babel for pickers compilation. I am not sure about building process in material-ui-x, but we are using rollup + Babel plugin which works just perfectly.

Tslib is actually Babel runtime helpers, but for native ts compiler.

I could contribute on making pretty same build process.

@oliviertassinari
Copy link
Member Author

I'm linking mui/material-ui#18447 from @eps1lon to the discussion, I think that it happens to be in the same space.

@eps1lon
Copy link
Member

eps1lon commented Jul 7, 2020

The grid is currently only compiled with tsc? How are different targets handled with tsc nowadays?

Otherwise I don't understand what the issue is. tslib is to tsc --importHelpers what @babel/runtime is to @babel/plugin-transform-runtime, no?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Jul 7, 2020

Otherwise I don't understand what the issue is. tslib is to tsc --importHelpers what @babel/runtime is to @babel/plugin-transform-runtime, no?

@eps1lon From what I understand, including babel/runtime and tslib in the bundle, leads to a duplication of the same helpers in the bundle. Does it increase the bundle size to a point we should care?

@eps1lon
Copy link
Member

eps1lon commented Jul 7, 2020

Does it increase the bundle size to a point we should care?

You opened the issue. I would've expected that you identified an issue.

If you need to transpile with babel and tsc (never encountered that use case. babel is usually enough) I'd be surprised if they would add the same helpers. In the end transpilation happens in a pipeline so if you transpile a piece of code with tsc babel wouldn't see it and transpile it again.

I never used tslib or tsc+babel so I can't help much without a full issue description.

@oliviertassinari
Copy link
Member Author

@eps1lon Thanks, I think that it will be interesting to follow what happens with mui/material-ui#18447, ideally, expose the same options here.

@zannager zannager added the core Infrastructure work going on behind the scenes label Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants