Better* defaults

[lukeed]

Addresses comments in #66 and #73. Went ahead with a PR just so that we can see what it'd look/feel like in action.

TLDR:

• Only enable compression & sourcemaps when desired
• Seems like most rely upon the --no-* flags (myself included) which were undocumented anyway
• Update snapshots' file sizes
• Add two fixtures: basic-with-compress, basic-with-sourcemap
• Update README docs

---

^* Biased

[texastoland]

For documentation I'd find Sade example()s more helpful then update the readme with the new --help. I'm willing to PR too.

Removing the default makes sense but I think it changes the common use case from --no-compress to --sourcemap instead of just works. Since it's a breaking change why not pursue your proposal in https://github.com/developit/microbundle/issues/66#issuecomment-361095723?

$ microbundle src
#=> default UMD is the only one minified and all with source maps

$ microbundle src --compress cjs --sourcemap cjs
#=> CJS is the only one minified with source maps

# maybe keep status quo if boolean

$ microbundle src --compress --sourcemap
#=> all minified with source maps

$ microbundle src --no-compress --no-sourcemap
#=> all unminified without source maps

After writing that I can't think of a use case for configuring source maps per format but false doesn't seem like the right default.

[lukeed]

Yeah, examples would be nice(r) -- just wanted to document before it was forgotten.

That comment was just a suggestion really. I agree with @Andarist's response in that it's a lot of extra processing & isn't really needed at the end of the day. I think compression & mapping should be all or nothing.

I PR'd what I'd personally like to see. Discussion can change it from here, but this is my proposal 😄

[developit]

Can we enable compression and sourcemaps for --target web by default? I know there's a bunch of people using Microbundle for Node, but it's still primarily used to bundle packages that get used by browsers.

For me, it's less about full minification (whitespace, etc), and more about running Uglify's optimizations on the bundled code. Perhaps instead of disabling compression, the default could be to enable beautification?

For me, I'd basically have to add --compress --source-map to every project I've ever used Microbundle in haha.

[ForsakenHarmony]

Why can't the user's bundler run the optimizations?

[developit]

That ignores the unpkg.com use-case. Also lots of folks don't have their bundlers configured to optimize to the same extent. In cases like Preact, nobody on earth has their bundler configured to optimize to the same extent (property minification, constant inlining, etc). Also it's slower to process 50kb of uncompressed source than it is to process 5kb of compressed source - comments and whitespace still have to be parsed.

I get that not everyone wants to minify things by default, but TBH that's kinda the whole point of this CLI. A stock rollup configuration could produce a single ES Module from a set of input modules. I think that's why I like the idea of --target web vs --target node - it means microbundle can serve both purposes without making sacrifices for either of them.

[lukeed]

I'd be happy with target specific settings 👍

Web

• Build:

  • compress: true
  • sourcemap: true

• Watch

  • compress: false
  • sourcemap: true

Node

• Build:

  • compress: false
  • sourcemap: false

• Watch:

  • compress: false
  • sourcemap: false

Those would be the default behaviors; specifying the --*/--no-* flags would override default setting.

[ForsakenHarmony]

But the preact esm dist is completely unminified

Having uglified source sucks when there's a problem in some lib and you're trying to figure out why

Sure there's unpkg.com but how many people actually use that for prod?

[ForsakenHarmony]

Maybe we should output both minified and unminified for the web target

[texastoland]

Having uglified source sucks when there's a problem in some lib and you're trying to figure out why

I see each of your perspectives but as a real world use case this point brought me to https://github.com/developit/microbundle/issues/66#issuecomment-412369258. My current personal project is Node but I'm typically front end. I've used public CDNs but never in production.

Maybe we should output both minified and unminified for the web target

It makes sense for Preact to distribute both compressed and non-compressed bundles. In that case maybe you could check the output for .min.js and run an extra pass in that case? Just an idea.

[maraisr]

@ForsakenHarmony @developit bump 🚀

[ForsakenHarmony]

I agree with luke, compression should be off by default, it's pretty much on @developit now

[developit]

Circling back to try to unblock this:

There are some transforms we currently use Terser for that genuinely can't be left off by default, especially if we produce both minified and unminified outputs. An example of this is property mangling: if we produce lib.min.js and lib.js, those two files should use the same properties - otherwise different bundler configurations will result in materially different object shapes being returned.

Here's my pitch: always run code through Terser, but in the "no compress" cases above, disable variable mangling and enable Terser's output.beautify option.

I've been using this for Preact debugging. It's still a nice improvement over debugging minified output, but it means I don't have to write code that needs to account for properties being "maybe" mangled.

Another option would be to simply move property mangling out of Terser and into Babel. If done in combination with moving Babel to transform bundles rather than source files, this would be relatively inexpensive. It would also allow for the un-minified output to preserve all of the semantics of the source modules, but strip comments and apply property compression to ensure the result minifies adequately in various bundlers.

Here's roughly what that plugin would look like:

import { transformAsync } from '@babel/core';

// a rollup output plugin
export class BabelFinalizer {
  async renderChunk(code, chunk) {
    const result = await transformAsync(code, {
      filename: chunk.filename,
      babelrc: false,
      configFile: false,
      plugins: [
        [manglePropertiesPlugin, minifyOptions]
      ]
    });
  }
}

export default function manglePropertiesPlugin({ types: t }) {
  function getMappedName(name, config) {
    if (!config.properties || !config.properties.regex) return;

    const reserved = config.properties.reserved || [];
    if (reserved.includes(name)) return;

    const reg = new RegExp(config.properties.regex);
    if (!reg.test(name)) return;
    
    let mapping = config.props;
    const keys = Object.keys(mapping);
    if (keys.length === 1 && keys[0] === 'props') {
      mapping = mapping.props;
    }
    
    const key = '$' + name;
    if (Object.prototype.hasOwnProperty.call(mapping, key)) {
      return mapping[key];
    }
  }
  return {
    name: 'babel-plugin-mangle-properties',
    visitor: {
      Identifier(path, state) {
        const config = state.opts;
        const parent = path.parentPath;
        
        if (!(
          (t.isObjectProperty(parent) && path.key === 'key') ||
          (t.isMemberExpression(parent) && path.key === 'property')
        )) return;

        const mapped = getMappedName(path.node.name, config);
        if (mapped != null) {
          path.replaceWith(t.identifier(mapped));
        }
      }
    }
  };
}

(see example transform)

[Andarist]

those two files should use the same properties - otherwise different bundler configurations will result in materially different object shapes being returned.

How is this a problem though? Bundlers should not pick both of such files at the same time and from what I understand property mangling can only be done for local shapes - otherwise it becomes an unsafe transform as it changes module's API.

[developit]

There is no way to accurately determine locality of property usage, short of building a complete partial evaluator like Closure, and even then it can't be done without additional information not present in JS syntax. Originally I started using this technique in Preact only for internal properties, as a way to intentionally make their names unreliable so that folks wouldn't use them however the usage has expanded since then to be more of a direct size optimization technique.

Your second point is the reason I pushed back on this: if property mangling is enabled for one output, it must be enabled for all outputs. The way it's commonly used, property mangling is equivalent to saying "please replace these symbols in my code with shorter ones" - it's naive, and doesn't even take into account the various runtime forms of property access/introspection like Object.defineProperties. It's not a pattern that makes sense for 100% of cases, but it should at least produce consistent results.

Longer-term, I would like to find a way to solve this using syntax, so that Microbundle is not changing the shape of objects but simply inlining already-minified property names.

Here are a few options currently in my head:

1. use symbols

A new mangle.symbols property could perform this, though the result would not be transparent since the compiled properties would be enumerable and their keys strings. However, the original constant definitions could be compiled and exported (as another entry), so that consumers of a given library could import them and use them to access the internal properties as if they were opaque symbols, even though they are compiled to short strings.

export const CHILDREN = Symbol.for('CHILDREN');
export const DOM = Symbol.for('DOM');
export const ORIGINAL = Symbol.for('ORIGINAL');

export function createVNode(type, props, original) {
  const vnode = {
    type,
    props,
    original,
    [CHILDREN]: null,
    [DOM]: null,
    [ORIGINAL]: original
  };
  if (!original) vnode[ORIGINAL] = vnode;
  return vnode;
}

2. Simple computed property aliases

Changing the semantics of Symbol.for() is scary. Instead, this would be trivial to implement since Terser handles all requisite inlining cases already. As with the first option, exporting the constants makes it possible to use these as more of a "protected" API surface where such a thing is necessary (an example is preact-devtools, which is inherently coupled to preact and the only consumer of many otherwise internal properties).

export const $children = '__k';
export const $dom = '__d';
export const $original = '__v';

export function createVNode(type, props, original) {
  const vnode = {
    type,
    props,
    original,
    [$children]: null,
    [$dom]: null,
    [$original]: original
  };
  if (!original) vnode[$original] = vnode;
  return vnode;
}

3. Introduce a helper

ECMAScript has private class fields, but not private object fields. We have Symbols for that, but Symbols don't compress well and are an unusual API surface. Introducing a faux-runtime helper could provide a way to avoid messing with the semantics of Symbol, while preserving that usage pattern:

import { property } from 'microbundle';

export const $children = property('children');
export const $dom = property('dom');
export const $original = property('original');

export function createVNode(type, props, original) {
  const vnode = {
    type,
    props,
    original,
    [$children]: null,
    [$dom]: null,
    [$original]: original
  };
  if (!original) vnode[$original] = vnode;
  return vnode;
}

---

Ultimately, I would love to have something better than any of these, where full escape analysis is done to determine when properties can be minified. However, even a Closure Compiler style analysis pass would still be missing the ability to define an exposed property as being internal.

[texastoland]

Hit this again after not using Microbundle in some time: https://twitter.com/texastoland/status/1504855004473360385?s=20&t=bbl1PuIFC76YV5kwx6u9DA

I can try to PR --target node's behavior and --no-compress in the readme (still poorly documented).