Skip to content

Commit

Permalink
Add babel plugin that removes code from @glimmer/debug that terser co…
Browse files Browse the repository at this point in the history
…uldn't figure out -- I also left some notes about the problem that terser is running in to -- it seems to be related to indirection of identity functions that 'passes' isn't smart enough to figure out

We get big wins (side-wise) from sideEffects: false, but things break.

This babel plugin is feeling more and more brittle, the more we want to
strip.
  • Loading branch information
NullVoxPopuli committed Sep 8, 2024
1 parent 287ab1a commit affee7b
Show file tree
Hide file tree
Showing 5 changed files with 149 additions and 162 deletions.
10 changes: 9 additions & 1 deletion packages/@glimmer-workspace/build/lib/config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* eslint-disable no-console */
// @ts-check
import { existsSync, readFileSync } from 'node:fs';
import { createRequire } from 'node:module';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';

Expand All @@ -12,6 +13,8 @@ import ts from 'typescript';

import inline from './inline.js';

const require = createRequire(import.meta.url);

// eslint-disable-next-line import/no-named-as-default-member
const { ModuleKind, ModuleResolutionKind, ScriptTarget, ImportsNotUsedAsValues } = ts;

Expand Down Expand Up @@ -94,7 +97,7 @@ export function typescript(pkg, config) {
return rollupTS({
transpiler: 'babel',
transpileOnly: true,
babelConfig: { presets },
babelConfig: { presets, plugins: [require.resolve('@glimmer/local-debug-babel-plugin')] },
/**
* This shouldn't be required, but it is.
* If we use @rollup/plugin-babel, we can remove this.
Expand Down Expand Up @@ -452,10 +455,15 @@ export class Package {

return {
input: resolve(root, ts),
treeshake: {
// moduleSideEffects: false,
moduleSideEffects: (id, external) => !external,
},
output: {
file: resolve(root, 'dist', env, file),
format,
sourcemap: true,
hoistTransitiveImports: false,
exports: format === 'cjs' ? 'named' : 'auto',
},
onwarn: (warning, warn) => {
Expand Down
1 change: 1 addition & 0 deletions packages/@glimmer-workspace/build/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"test:types": "tsc --noEmit -p ../tsconfig.json"
},
"dependencies": {
"@glimmer/local-debug-babel-plugin": "workspace:*",
"@rollup/plugin-commonjs": "^25.0.7",
"@rollup/plugin-node-resolve": "^15.2.3",
"@rollup/plugin-replace": "^5.0.5",
Expand Down
110 changes: 110 additions & 0 deletions packages/@glimmer/local-debug-babel-plugin/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/**
* https://astexplorer.net/#/gist/c3f41b75af73006f64476775e73f7daa/e6e3e120df8404b1bff308bec3ed89eaaf0b05f2
*
* This plugin exists because, _even when we inline @glimmer/debug_,
* we cannot get terser to remove/inline/unwrap identity functions.
*
* Repro here: https://try.terser.org/
*
* ```js
function x(x) { return x; }
function y(x, y, z) { return x; };
function abc(a) { return x(a); }
function abc2(a) { return y(a); }
export function example() {
return `${x(2)} ${y("2")} ${abc(3)} ${abc2("3")}`;
}
```
With Options:
{
module: true,
compress: {
passes: 6,
module: true,
inline: 3, // default
},
mangle: {},
output: {},
parse: {},
rename: {},
}
*/
export default function (babel) {
let _removeCheck = removeChecks(babel);

return {
name: 'Cleanup local-debug code that terser could not',
visitor: {
ImportDeclaration(path, state) {
_removeCheck.ImportDeclaration(path, state);
},
CallExpression(path, state) {
_removeCheck.CallExpression(path, state);
},
},
};
}

function removeChecks({ template }) {
let stateKey = Symbol.for('removeChecks');

const unwrap = ['check'];
const trueFn = ['CheckInterface', 'wrap', 'CheckOr', 'CheckFunction', 'CheckObject'];
const removeEntirely = ['recordStackSize'];

function isToBeRemoved(callPath, state) {
if (!state) return;

let ourState = state[stateKey];

if (!ourState) return;
/**
* Do we want to support local aliasing / re-assignment?
* if so, this would break
*/
if (!ourState?.names?.has(callPath.node.callee.name)) return;

return true;
}

return {
ImportDeclaration(path, state) {
let node = path.node;

if (!node.source) return;
if (node.source.value !== '@glimmer/debug') return;

state[stateKey] ??= { names: new Set(), nodes: [] };

node.specifiers.forEach((specifier) => {
let name = specifier.local.name;
let relevant =
unwrap.includes(name) || removeEntirely.includes(name) || trueFn.includes(name);
if (!relevant) return;

state[stateKey].names.add(name);
state[stateKey].nodes.push(specifier.local);
});
},
CallExpression(path, state) {
if (isToBeRemoved(path, state)) {
let name = path.node.callee.name;
if (removeEntirely.includes(name)) {
path.remove();
return;
}

if (trueFn.includes(name)) {
path.replaceWith(template(`() => true`)());
return;
}

path.replaceWith(path.node.arguments[0]);
}
},
};
}
13 changes: 13 additions & 0 deletions packages/@glimmer/local-debug-babel-plugin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@glimmer/local-debug-babel-plugin",
"version": "0.92.0",
"license": "MIT",
"repository": {
"type": "git",
"url": "https://github.com/glimmerjs/glimmer-vm.git",
"directory": "packages/@glimmer/local-debug-babel-plugin"
},
"type": "module",
"private": true,
"main": "index.js"
}
Loading

0 comments on commit affee7b

Please sign in to comment.