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

Add UMD build #59

Merged
merged 6 commits into from
Dec 8, 2019
Merged

Add UMD build #59

merged 6 commits into from
Dec 8, 2019

Conversation

perliedman
Copy link
Contributor

@perliedman perliedman commented Oct 5, 2019

Hi, and thanks for a very nice component library!

Since I am still in an old build setup, I rely on libraries being bundled up as UMD or IIFE. I have added an IIFE build to the project to support this, and I guess it might be of use to other people in my situation.

This change bundles the component together with the project's dependencies, but leaves out the peer dependencies, which I guess makes most sense for a standalone browser bundle.

I had to make an inelegant hack to solve a problem where global for some reason is not properly polyfilled in the IIFE build - I am too inexperienced with Babel to understand why this is, but it seems other people have the same problem: rollup/rollup-plugin-commonjs#6 (comment) - please tell me if you know of a better way to solve this (or if there is anything else that needs to be addressed in this PR, of course!)

@perliedman
Copy link
Contributor Author

I notice DeepScan reporting an error, but from what I can tell it is not related to the changes I made. Is it possible that the warning was already there before my PR?

@gilbarbara
Copy link
Owner

Hey,
Thanks for the PR.
You are the first person to talk about UMD/IIFE in all my packages, so I don't think a lot of people still use it. How do you optimize bundles? 😜

Anyway, I'm rewriting this library with TypeScript and I'll stop using rollup.
I don't think it's possible to generate IIFE with it, so can you update the setup to use UMD?

And the DeepScan error is a new check they've added, so not related to this PR.

@perliedman perliedman changed the title Add IIFE build Add UMD build Oct 7, 2019
@perliedman
Copy link
Contributor Author

Yeah, I know it's a bit old school to bundle things like that, and regarding optimizing bundles: we don't 😄 I mean, the bundle ends up with whatever duplication our dependencies happen to include (and surprisingly many libs still include IIFE/UMD builds). Far from ideal, and definitely one of the things I'm less happy about, but for now I just need to deal with it.

Anyway, I converted to UMD, it works for my very limited test.

@gilbarbara gilbarbara merged commit 35c5810 into gilbarbara:master Dec 8, 2019
@ashubham ashubham mentioned this pull request Sep 1, 2020
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.

2 participants