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

feat(config): allow override of rollup & plugins config by project config #755

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

olance
Copy link

@olance olance commented Nov 21, 2020

I love the simplicity of microbundle, but I need to do a bit more than what the default configuration it builds up provides.

More specifically, I want to be able to import SVG images.
Issue #679 says "for that we recommend going with rollup directly and writing a config for your needs.".

Given everything microbundle does for me, having to let it go and configure rollup myself just because of some SVG files makes me quite sad, so I pulled up this PoC of how microbundle could allow some customization of its build config.

@marvinhagemeister as you were the one making that statement in the SVG issue, maybe you'd be able to review/comment this?

It's quite "quick and dirty" for the moment, but I first wanted to know whether that could be a direction microbundle could take?

As a concrete example, that PR allows me to add a microbundle.config.js file into my project and support importing SVG files by importing the @rollup/plugin-image plugin:

const image = require('@rollup/plugin-image')

module.exports = {
  plugins: {
    typescript: function (config) {
      // Since imported *.svg will be transformed to ES modules by the image plugin, tell
      // TypeScript to process them
      return { ...config, include: ['*.ts+(|x)', '**/*.ts+(|x)', '*.svg', '**/*.svg'] }
    },
  },

  config: function (config) {
    // Add image plugin right after json plugin
    const jsonIdx = config.inputOptions.plugins.findIndex((p) => p.name === 'json')
    config.inputOptions.plugins.splice(jsonIdx + 1, 0, image())
    return config
  },
}

@changeset-bot
Copy link

changeset-bot bot commented Nov 21, 2020

⚠️ No Changeset found

Latest commit: f149b88

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@olance
Copy link
Author

olance commented Dec 6, 2020

Any feedback on this? @developit maybe?

@developit
Copy link
Owner

developit commented Dec 17, 2020

Hi @olance! I haven't had a chance to review the code here yet, but I am struggling with the philosophical/directional impact.

Microbundle was created to serve a very specific need: bundling libraries before they are published to npm. I know lots of folks are using it for much more than that, but I don't actually think it serves other use-cases very well. It doesn't provide hot module replacement, bakes in relatively opinionated default output configurations, includes default plugins that can't be un-defaulted, compiles code in an aggressively loose configuration, etc. These things all make sense for bundling libraries, but they're not really great defaults for apps or websites, and they would make configuration destructive (as you pointed out with the plugin ordering snippet, plugins have to be aware of other plugins).

On the flip side, we've recently released WMR. It's a standalone application development tool that uses Rollup to bundle for production and supports Rollup plugins (during development too, despite not bundling your source code). Since WMR is maintained by roughly the same group of people who maintain Microbundle, it feels like we might be better off transitioning folks currently using Microbundle as an app bundler over to WMR.

Microbundle derives immense benefit from not offering configuration, because it means we don't leak the details of dependencies like Rollup to users. This allows controlling versions and compatibility, which is what makes it possible to upgrade microbundle across many projects without breaking configurations. One example to ponder: it is very likely that Microbundle's Babel + TypeScript setup will change in the near future, as part of improving performance by reducing the number of AST passes over source code. If we allow configuration to specify options that are passed into rollup-plugin-typescript2 and @rollup/babel-plugin, those plugins become part of our public-facing API. Switching out how we process typescript is no longer an implementation detail, and is instead a breaking change.

Ok, coming back to libraries (perhaps you're wanting to bundle these SVG's into a library?). One of the things I'm stuck on with this PR is that it seems like the example could be addressed in a generalized way without introducing configuration. Looking at your specific SVG use-case, what if Microbundle were to offer a text: import prefix that returned the text content of the filepath it were applied to?

import svg from 'text:./icon.svg';
     // ^ "<svg ....</svg>"

export const Icon = () => <span className="icon" dangerouslySetInnerHTML={{ __html: svg }} />;

@keul
Copy link

keul commented Dec 17, 2020

@developit whatever will be the final implementation what is missing is quite clear: bundling a resource with a package (an image, or CSS background image, or a font) using microbundle is very hard (or not possible).

@marvinhagemeister
Copy link
Collaborator

@keul To give some context: That scenario is hard because microbundle was never build to bundle resources. It was born out of our main Preact repo where folks always asked us if we could turn our build pipeline into something that could be re-used for other libraries.

@developit
Copy link
Owner

Indeed. One thing I forgot to mention in my reply that might help clarify: the original reason Microbundle did not include a way to bundle non-JS resources is because there's no standardized mechanism to do so. The closest thing we have to a standard for importing non-JS resources is a few bundlers using a similar import prefix technique.

Then there's the issue of bloat: it's certainly most convenient to bundle images or fonts into JS when publishing to npm, but the performance impact of doing so is surprisingly dire. Images and fonts are binary data that must be Base64-encoded to embed in JS as strings. Embedding a 300kb font in an npm module will increase everyone's bundle size by 1mb, because Base64 incurs approximately 2x overhead. Worse still, it's not just the file size that becomes a problem - 1mb of image data in a JavaScript bundle incurs parsing and execution cost, because there is no way for a JS engine to know a given string is 1mb of external data.

Since Microbundle's primary goal is to help folks bundle modules in a way that keeps them small and fast, making it easy to explode bundle sizes sortof goes against the project's purpose.

That said, I totally agree there's a need for something here. The core of the problem is that there is no accepted way to refer to resources like images or fonts from within an npm module, which is what leads folks to go the embedding route. Maybe we can change that? In a pure ES Modules world, the follow code works:

const img = document.createElement('img');
img.src = new URL('./my-image.svg', import.meta.url);
document.body.appendChild(img);

The SVG is kept out of JavaScript, and bundlers can transform or statically evaluate the URL() construction. I believe Webpack is now doing this by default in version 5, which gives me hope that this could work in practise.

@keul
Copy link

keul commented Dec 17, 2020

Thanks for your point on this.

I perfectly understand the issue, my general concern was exactly about this: no shared way of doing this, although usecases where external a JavaScript library should provide also a resource (or a CSS) is quite common today.

@developit
Copy link
Owner

Yeah, CSS really seems like the #1 pain point here. As part of thinking about this, I've opened preactjs/wmr#260 to land support for bundling the above technique there (if it's not already supported, which is might be? haha).

@developit developit added the increased scope Increases project scope, or is out of scope. label Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
increased scope Increases project scope, or is out of scope.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants