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): closure compiler for minification #525

Closed
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,8 @@ export interface TsdxOptions {
writeMeta?: boolean;
// Only transpile, do not type check (makes compilation faster)
transpileOnly?: boolean;
// EXPERIMENTAL: Use closure compiler to minify production bundle instead of terser
closureCompiler?: boolean;
}
```

Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"templates"
],
"dependencies": {
"@ampproject/rollup-plugin-closure-compiler": "^0.22.2",
"@babel/core": "^7.4.4",
"@babel/helper-module-imports": "^7.0.0",
"@babel/plugin-proposal-class-properties": "^7.4.4",
Expand Down
27 changes: 15 additions & 12 deletions src/createRollupConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { safeVariableName, safePackageName, external } from './utils';
import { paths } from './constants';
import { RollupOptions } from 'rollup';
import { terser } from 'rollup-plugin-terser';
import closureCompiler from '@ampproject/rollup-plugin-closure-compiler';
import { DEFAULT_EXTENSIONS } from '@babel/core';
// import babel from 'rollup-plugin-babel';
import commonjs from '@rollup/plugin-commonjs';
Expand Down Expand Up @@ -195,18 +196,20 @@ export async function createRollupConfig(
// printInfo: false,
// }),
shouldMinify &&
terser({
sourcemap: true,
output: { comments: false },
compress: {
keep_infinity: true,
pure_getters: true,
passes: 10,
},
ecma: 5,
toplevel: opts.format === 'cjs',
warnings: true,
}),
(opts.closureCompiler
? closureCompiler()
: terser({
sourcemap: true,
output: { comments: false },
compress: {
keep_infinity: true,
pure_getters: true,
passes: 10,
},
ecma: 5,
toplevel: opts.format === 'cjs',
warnings: true,
})),
],
};
}
1 change: 1 addition & 0 deletions src/env.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ declare module '@babel/core' {
}

// Rollup plugins
declare module '@ampproject/rollup-plugin-closure-compiler';
declare module '@jaredpalmer/rollup-plugin-preserve-shebang';
ambroseus marked this conversation as resolved.
Show resolved Hide resolved
declare module 'rollup-plugin-babel';
declare module 'rollup-plugin-size-snapshot';
Expand Down
5 changes: 5 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,11 @@ prog
.example('build --tsconfig ./tsconfig.foo.json')
.option('--transpileOnly', 'Skip type checking')
.example('build --transpileOnly')
.option(
'--closureCompiler',
'EXPERIMENTAL: Use closure compiler to minify production bundle instead of terser'
)
.example('build --closureCompiler')
.option(
'--extractErrors',
'Extract errors to ./errors/codes.json and provide a url for decoding.'
Expand Down
2 changes: 2 additions & 0 deletions src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export interface TsdxOptions extends SharedOpts {
writeMeta?: boolean;
// Only transpile, do not type check (makes compilation faster)
transpileOnly?: boolean;
// EXPERIMENTAL: Use closure compiler to minify production bundle instead of terser
closureCompiler?: boolean;
}

export interface PackageJson {
Expand Down
7 changes: 0 additions & 7 deletions test/fixtures/build-withConfig/errors/ErrorDev.js

This file was deleted.

14 changes: 0 additions & 14 deletions test/fixtures/build-withConfig/errors/ErrorProd.js

This file was deleted.

4 changes: 0 additions & 4 deletions test/fixtures/build-withConfig/errors/codes.json

This file was deleted.

11 changes: 11 additions & 0 deletions test/fixtures/build-withConfig/src/closure.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const split = (str: string) => str.split('');

const sum = (a: number, b: number) => {
return a + b;
};

const bar = split('bar');

// this line gets minified differently with
// Terser vs. Closure vs. Closure ADVANCED_OPTIMIZATIONS
export const signature = `${split('bar').join('')} ${sum(bar.length, -3)}`;
1 change: 0 additions & 1 deletion test/fixtures/build-withConfig/src/foo.ts

This file was deleted.

6 changes: 1 addition & 5 deletions test/fixtures/build-withConfig/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import invariant from 'tiny-invariant';
import warning from 'tiny-warning';
invariant(true, 'error occurred! o no');
warning(false, 'warning - water is wet');
export { foo } from './foo';
export { signature } from './closure';

export const sum = (a: number, b: number) => {
if ('development' === process.env.NODE_ENV) {
Expand Down
19 changes: 19 additions & 0 deletions test/fixtures/build-withConfig/tsdx.config.closure-advanced.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
const closureCompiler = require('@ampproject/rollup-plugin-closure-compiler');

const closureCompilerPlugin = closureCompiler({
compilation_level: 'ADVANCED_OPTIMIZATIONS',
});

module.exports = {
rollup(config) {
config.plugins = config.plugins.map(plugin => {
// override closure compiler plugin's default config
if (plugin && plugin.name === closureCompilerPlugin.name) {
return closureCompilerPlugin;
}
return plugin;
});

return config;
},
};
22 changes: 0 additions & 22 deletions test/fixtures/build-withConfig/tsdx.config.js

This file was deleted.

12 changes: 12 additions & 0 deletions test/fixtures/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ const rootDir = process.cwd();

shell.config.silent = true;

// shelljs.grep wrapper
// @param {string|RegExp} pattern
// @param {string} fileName
// @returns {boolean} true if pattern has matches in file
function grep(pattern, fileName) {
const output = shell.grep(pattern, fileName);
// output.code is always 0 regardless of matched/unmatched patterns
// so need to test output.stdout
return Boolean(output.stdout.match(pattern));
}
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved

module.exports = {
setupStageWithFixture: (stageName, fixtureName) => {
const stagePath = path.join(rootDir, stageName);
Expand All @@ -22,5 +33,6 @@ module.exports = {
shell.rm('-rf', path.join(rootDir, stageName));
},

grep,
rootDir,
};
70 changes: 70 additions & 0 deletions test/tests/tsdx-build-closure-compiler.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/**
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved
* @jest-environment node
*/

const shell = require('shelljs');
const util = require('../fixtures/util');

shell.config.silent = false;

const stageName = 'stage-build-closure-compiler';

const cjsBundleDev = 'dist/build-withconfig.cjs.development.js';
const cjsBundleProd = 'dist/build-withconfig.cjs.production.min.js';
const esmBundle = 'dist/build-withconfig.esm.js';
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved

const simplePattern = /exports\.signature=signature/;
const advancedPattern = /exports\.a="bar 0"/;
Copy link
Collaborator

Choose a reason for hiding this comment

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

huh... doesn't this minification break the usage?? since signature is no longer exported. is that a bug in closure or is there a way to disable exports minification?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@agilgur5 hmm.. this is not good.. need to investigate (a lot of ADVANCED options)

Copy link
Collaborator

@agilgur5 agilgur5 Mar 29, 2020

Choose a reason for hiding this comment

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

Ah, I haven't looked the API at all. Keywords would likely be "exports" and "mangle" as I believe the technical term for changing variable names is "mangling"

At least we're already iterating on some future defaults 😅


describe('tsdx build with closure compiler', () => {
beforeAll(() => {
util.teardownStage(stageName);
});

it('should minify bundle with default options', () => {
util.setupStageWithFixture(stageName, 'build-withConfig');

let output = shell.exec('node ../dist/index.js build --closureCompiler');
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

expect(output.code).toBe(0);

expect(shell.test('-f', 'dist/index.js')).toBeTruthy();
expect(shell.test('-f', 'dist/index.d.ts')).toBeTruthy();
expect(shell.test('-f', cjsBundleDev)).toBeTruthy();
expect(shell.test('-f', cjsBundleProd)).toBeTruthy();
expect(shell.test('-f', esmBundle)).toBeTruthy();

const lib = require(`../../${stageName}/dist`);
expect(lib.signature).toBe('bar 0');

// with SIMPLE optimization level minified bundle should contain
// simplePattern 'exports.signature=signature'
expect(util.grep(simplePattern, cjsBundleProd)).toBeTruthy();
expect(util.grep(advancedPattern, cjsBundleProd)).toBeFalsy();
agilgur5 marked this conversation as resolved.
Show resolved Hide resolved
});
ambroseus marked this conversation as resolved.
Show resolved Hide resolved

it('should minify bundle with advanced options', () => {
util.setupStageWithFixture(stageName, 'build-withConfig');
shell.mv('-f', 'tsdx.config.closure-advanced.js', 'tsdx.config.js');

let output = shell.exec('node ../dist/index.js build --closureCompiler');
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

expect(output.code).toBe(0);

expect(shell.test('-f', 'dist/index.js')).toBeTruthy();
expect(shell.test('-f', 'dist/index.d.ts')).toBeTruthy();
expect(shell.test('-f', cjsBundleDev)).toBeTruthy();
expect(shell.test('-f', cjsBundleProd)).toBeTruthy();
expect(shell.test('-f', esmBundle)).toBeTruthy();

const lib = require(`../../${stageName}/dist`);
expect(lib.signature).toBe('bar 0');

// with ADVANCED optimization level minified bundle should contain
// advancedPattern 'exports.a="bar 0"'
expect(util.grep(simplePattern, cjsBundleProd)).toBeFalsy();
expect(util.grep(advancedPattern, cjsBundleProd)).toBeTruthy();
});
ambroseus marked this conversation as resolved.
Show resolved Hide resolved

afterEach(() => {
util.teardownStage(stageName);
});
});
Loading