-
Notifications
You must be signed in to change notification settings - Fork 361
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
Better* defaults #187
base: master
Are you sure you want to change the base?
Better* defaults #187
Conversation
For documentation I'd find Sade Removing the default makes sense but I think it changes the common use case from $ 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 |
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 😄 |
Can we enable compression and sourcemaps for 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 |
Why can't the user's bundler run the optimizations? |
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 |
I'd be happy with Web
Node
Those would be the default behaviors; specifying the |
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? |
Maybe we should output both minified and unminified for the web target |
I see each of your perspectives but as a real world use case this point brought me to #66 (comment). My current personal project is Node but I'm typically front end. I've used public CDNs but never in production.
It makes sense for Preact to distribute both compressed and non-compressed bundles. In that case maybe you could check the output for |
@ForsakenHarmony @developit bump 🚀 |
I agree with luke, compression should be off by default, it's pretty much on @developit now |
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 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) |
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. |
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 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 symbolsA new 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 aliasesChanging 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 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 helperECMAScript 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. |
Hit this again after not using Microbundle in some time: I can try to PR |
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:
--no-*
flags (myself included) which were undocumented anywaybasic-with-compress
,basic-with-sourcemap
* Biased