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

Build Modernization #1271

Closed
ChristopherBiscardi opened this issue Sep 18, 2020 · 6 comments
Closed

Build Modernization #1271

ChristopherBiscardi opened this issue Sep 18, 2020 · 6 comments
Labels
🏗 area/tools This affects tooling 💪 phase/solved Post is done 🦋 type/enhancement This is great to have

Comments

@ChristopherBiscardi
Copy link
Member

#1269 introduces microbundle to handle the preact package builds and enables node v14 native esm usage for mdx (that is, importing mdx from .mjs files). The usage should expand and result in the removal of @pkgr/rollup.

This issue should be closed after the work in #1269 is expanded to the other packages.

@wooorm
Copy link
Member

wooorm commented Nov 13, 2020

@ChristopherBiscardi Is this, to bring ESM to all packages, something you can work on, or should others work on it?

@ChristianMurphy
Copy link
Member

Cross pollinating, micromark recently went through a similar ESM modularization.
micromark/micromark#27, micromark/micromark#28, micromark/micromark#35, micromark/micromark#36.
The transition worked well for node projects, but runtime/browser usage (particularly builds with webpack 4) was negatively impacted remarkjs/react-markdown#518

I'm all for a modular build, there may be some additional documentation on supported build systems, and/or testing common build platforms which may be needed to roll this out smoothly.

@ChristopherBiscardi
Copy link
Member Author

To clarify the core purpose of this issue: we are currently on an unsupported one-off build toolchain that isn't used anywhere else afaict, was last released months ago, and has only 257 weekly downloads: https://www.npmjs.com/package/@pkgr/rollup. It is a project risk issue to depend on a niche build pipeline that we don't control and isn't maintained. The work was started to get rid of that toolchain and put us onto something more mainstream that is actively maintained (microbundle in this case) that is still based on rollup.

Your concerns about the handling of esm conversion in micromark are heard, but not required for this conversion as the source code is not changing and all existing exports are being maintained as-is. I'll repeat that we are not changing the underlying build system like micromark did (from browserify to rollup). MDX is already built using rollup.

@wooorm
Copy link
Member

wooorm commented Dec 23, 2020

I replaced pkgr with microbundle for vue and react already: e7e9d46. It’s no longer used.

MDX is already built using rollup.

Not all of it is: mdx-js/mdx, mdx-js/loader , and mdx-js/vue-loader use CJS. I was wondering if this issue was also about changing them to MJS?

(aside: mdx-js/runtime uses MJS but doesn’t have microbundle set up yet so that’s a todo, it seems it was done by pkgr before and I missed it)

wooorm added a commit that referenced this issue Dec 23, 2020
johno pushed a commit that referenced this issue Dec 23, 2020
@ChristopherBiscardi
Copy link
Member Author

I replaced pkgr with microbundle for vue and react already: e7e9d46. It’s no longer used.

great! seems like this would've been a useful issue to mention in that PR.

MDX is already built using rollup.

Not all of it is: mdx-js/mdx, mdx-js/loader , and mdx-js/vue-loader use CJS.

you are correct in stating that the packages that don't get built, are also not built with rollup. I will be more clear about that next time.

I was wondering if this issue was also about changing them to MJS?

This issue is about removing pkgr. If pkgr is gone, we can close this issue.

@wooorm
Copy link
Member

wooorm commented Dec 27, 2020

Perfect, and yes you’re right!

Then this was closed by GH-1338.

@wooorm wooorm closed this as completed Dec 27, 2020
@wooorm wooorm added 🏗 area/tools This affects tooling 💪 phase/solved Post is done and removed 🔍 status/open labels Dec 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗 area/tools This affects tooling 💪 phase/solved Post is done 🦋 type/enhancement This is great to have
Development

No branches or pull requests

3 participants