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

Use microbundle standard packaging #445

Merged
merged 8 commits into from
Mar 11, 2019
Merged

Use microbundle standard packaging #445

merged 8 commits into from
Mar 11, 2019

Conversation

atomiks
Copy link
Owner

@atomiks atomiks commented Mar 11, 2019

This doesn't switch to microbundle, but uses microbundle packaging fields as suggested in README.

Adds unpkg field. The code in the main field should be unminified.

Note: It doesn't use the same directory format, microbundle uses dist/* with format file names. We're using directories here because there are a bunch of different formats and it's easier to import this way.

import tippy from 'tippy.js/esm' => import without CSS

Instead of:

import tippy from 'tippy.js/dist/tippy.mjs' => 😕

@atomiks atomiks requested a review from KubaJastrz March 11, 2019 03:40
@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

I wonder if we should also run prettier over the dist files, because the output isn't very pretty. Although sourcemaps kind of alleviate that anyway

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

I wonder if we should also support a "modern" package that uses ES2017 syntax (the one where <script type="module"> was introduced, for smaller bundle sizes for setups that are using dual build dist files.

Also I think we need to update the index.d.ts to declare these modules, because VSCode can't find the files for these types of imports.

Copy link
Collaborator

@KubaJastrz KubaJastrz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool 👍

rollup.build.js Outdated Show resolved Hide resolved
@KubaJastrz
Copy link
Collaborator

Also I think we need to update the index.d.ts to declare these modules

It can be done with declare module.

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

It can be done with declare module.

@KubaJastrz can you push to this branch with this?

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

After thinking about it more, going to remove the cjs bundle. UMD works the same as CJS in Node envs anyway with a few extra bytes. But I'd still like "unpkg" field and unminified code in "main".

Also we need to support browser-only esm, because import Popper from 'popper.js' does not work in a browser. But I'd like to avoid bundling it :\

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

Browser ESM version

I'm thinking we can have:

./esm/index.all.browser.js (+ minified)

No bundling of Popper.js. Some post-rollup build step can just replace this line:

- import Popper from 'popper.js'
+ import Popper from 'https://unpkg.com/popper.js@1/dist/esm/popper.min.js'

Same extra HTTP request as the UMD version. And users can still import Popper.js themselves without extra size. And it won't become outdated.

Modern version (ES2017)

./esm/index.all.modern.js (+ minified)

For users to place in bundles that use type="module". This will just use a custom browserslist config, and CSS without any -webkit- prefix transforms.

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

Okay the "modern mode" is pointless because it doesn't keep the default params/destructuring (looking around, seems like Edge didn't support it for a while). So it ended up being like 5 bytes larger 🤦‍♂️

I've added a browser ESM version though

@atomiks
Copy link
Owner Author

atomiks commented Mar 11, 2019

Turns out unpkg already does browser ESM properly, although it's not minified. Seems to be an open thread on it though. Gonna close this out... 😪

@atomiks atomiks merged commit f0fc58f into master Mar 11, 2019
@atomiks atomiks deleted the microbundle-package branch March 11, 2019 12:41
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.

None yet

2 participants