Skip to content

Commit

Permalink
(chore) - remove closure-compiler (#1570)
Browse files Browse the repository at this point in the history
* remove closure-compiler, the current rollup plugin isn't really maintained and we were seeing frequent issues with metro and bundles compiled by closure

* add changeset

* Update curvy-bobcats-fry.md

* Remove hoist options from Terser

* Remove version from ephemeral package.json

* Remove babel-plugin-closure-elimination

* Add transformation to clean up function expressions

Buble/Babel (and transformers in general) like to transform
arrow functions to function expressions on variable declarations.
At the toplevel of modules we can clean this up by replacing them
with function declarations, which leads to cleaner minifier output.

* Expand function expression transformer to more safe cases

Co-authored-by: Phil Pluckthun <[email protected]>
  • Loading branch information
JoviDeCroock and kitten authored Apr 28, 2021
1 parent 398007e commit 25e6c5b
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 159 deletions.
22 changes: 22 additions & 0 deletions .changeset/curvy-bobcats-fry.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
'@urql/exchange-auth': patch
'@urql/exchange-execute': patch
'@urql/exchange-graphcache': patch
'@urql/exchange-multipart-fetch': patch
'@urql/exchange-persisted-fetch': patch
'@urql/exchange-populate': patch
'@urql/exchange-refocus': patch
'@urql/exchange-request-policy': patch
'@urql/exchange-retry': patch
'@urql/exchange-suspense': patch
'@urql/core': patch
'@urql/introspection': patch
'next-urql': patch
'@urql/preact': patch
'urql': patch
'@urql/storybook-addon': patch
'@urql/svelte': patch
'@urql/vue': patch
---

Remove closure-compiler from the build step
3 changes: 2 additions & 1 deletion exchanges/graphcache/default-storage/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"name": "urql-exchange-graphcache-default-storage",
"private": true,
"version": "0.0.0",
"main": "../dist/urql-exchange-graphcache-default-storage",
"module": "../dist/urql-exchange-graphcache-default-storage.mjs",
"types": "../dist/types/default-storage/index.d.ts",
Expand All @@ -15,7 +16,7 @@
"./package.json": "./package.json"
},
"dependencies": {
"@urql/core": ">=1.16.0",
"@urql/core": ">=2.0.0",
"wonka": "^4.0.14"
}
}
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
"react-is": "^17.0.1"
},
"devDependencies": {
"@ampproject/rollup-plugin-closure-compiler": "^0.26.0",
"@babel/core": "^7.12.13",
"@babel/plugin-transform-object-assign": "^7.12.13",
"@babel/plugin-transform-react-jsx": "^7.12.13",
Expand All @@ -60,7 +59,6 @@
"@types/jest": "^26.0.20",
"@typescript-eslint/eslint-plugin": "^4.22.0",
"@typescript-eslint/parser": "^4.22.0",
"babel-plugin-closure-elimination": "^1.3.2",
"babel-plugin-modular-graphql": "1.0.1",
"babel-plugin-transform-async-to-promises": "^0.8.15",
"dotenv": "^8.2.0",
Expand Down
95 changes: 95 additions & 0 deletions scripts/babel/transform-function-expressions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/** Babel plugin for cleaning up arrow function transpilation, which turns function expressions assigned to variable decalators into function declarations when it's safe to do so. */
const functionExpressionCleanup = ({ types: t }) => {
/** Checks whether this block has only safe conditions up until the given node. */
const isSafeUntil = (block, until) => {
let body = [];
if (t.isIfStatement(block)) {
body = block.consequent;
if (block.alternate && !isSafeUntil(block.alternate, until)) {
return false;
}
} else if (t.isBlockStatement(block)) {
body = block.body;
}

for (let i = 0, l = body.length; i < l; i++) {
let node = body[i];
if (t.isIfStatement(node)) {
// An if statement is safe if it also is safe throughout
if (!isSafeUntil(node, until)) return false;
} else if (
!t.isVariableDeclaration(node) &&
!t.isFunctionDeclaration(node) &&
!(t.isExpressionStatement(node) && t.isAssignmentExpression(node.expression))
) {
// only variable declarations and function declarations are safe
// assignments are fine too, since we're later checking the binding for "constantViolations"
return false;
} else if (node === until) {
return true;
}
}

return true;
};

return {
visitor: {
FunctionExpression(path) {
if (!t.isVariableDeclarator(path.parent)) {
// Must be on a variable declarator
return;
}

if (
t.isFunctionDeclaration(path.parentPath.scope.block) ||
t.isFunctionExpression(path.parentPath.scope.block)
) {
// When the function expression is nested inside another function, it may be safe
// to turn this into a declaration, if it's only preceded by variable declarations
// and assignments (potentially even nested in if-statements)
if (!isSafeUntil(path.parentPath.scope.block.body, path.parentPath.parent))
return;
} else if (!t.isProgram(path.parentPath.scope.block)) {
return;
}

const binding = path.scope.getBinding(path.parent.id.name);

if (
(binding.constantViolations && binding.constantViolations.length) ||
binding.referencePaths.some(path =>
!t.isCallExpression(path.parentPath.node) &&
!t.isProgram(path.parentPath.node))
) {
// The declaration must not be reassigned and it must only be referenced as plain calls
return;
}

const fn = t.functionDeclaration(
path.parent.id,
path.node.params,
path.node.body,
path.node.generator,
path.node.async,
);

// We insert after other variable declarators to not rely on hoisting and for readability
path.parentPath.parentPath.insertAfter(
// If the variabe is exported then the function declaration must also be exported.
t.isExportNamedDeclaration(path.parentPath.parentPath.parent)
? t.exportNamedDeclaration(fn)
: fn
);

if (path.parentPath.parent.declarations.length <= 1) {
path.parentPath.parentPath.remove();
} else {
path.remove();
}
}
}
};
};

export default functionExpressionCleanup;
1 change: 1 addition & 0 deletions scripts/rollup/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const input = settings.sources.reduce((acc, source) => {
baseContents: {
name: source.name,
private: true,
version: '0.0.0',
main: join(rel, dirname(source.main), basename(source.main, '.js')),
module: join(rel, source.module),
types: join(rel, source.types),
Expand Down
19 changes: 9 additions & 10 deletions scripts/rollup/plugins.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ import sucrase from '@rollup/plugin-sucrase';
import buble from '@rollup/plugin-buble';
import replace from '@rollup/plugin-replace';
import babel from '@rollup/plugin-babel';
import compiler from '@ampproject/rollup-plugin-closure-compiler';
import visualizer from 'rollup-plugin-visualizer';
import { terser } from 'rollup-plugin-terser';

import cleanup from './cleanup-plugin.js'
import babelPluginTransformFunctionExpressions from '../babel/transform-function-expressions';
import babelPluginTransformPipe from '../babel/transform-pipe';
import babelPluginTransformInvariant from '../babel/transform-invariant-warning';
import babelPluginTransformDebugTarget from '../babel/transform-debug-target';
Expand Down Expand Up @@ -75,7 +75,7 @@ export const makePlugins = () => [
babelPluginTransformDebugTarget,
babelPluginTransformPipe,
babelPluginTransformInvariant,
'babel-plugin-closure-elimination',
babelPluginTransformFunctionExpressions,
'@babel/plugin-transform-object-assign',
settings.hasReact && ['@babel/plugin-transform-react-jsx', {
pragma: 'React.createElement',
Expand Down Expand Up @@ -104,10 +104,6 @@ export const makeOutputPlugins = ({ isProduction, extension }) => {
isProduction && replace({
'process.env.NODE_ENV': JSON.stringify('production')
}),
!settings.mayReexport && compiler({
formatting: 'PRETTY_PRINT',
compilation_level: 'SIMPLE_OPTIMIZATIONS'
}),
cleanup({ extension }),
isProduction ? terserMinified : terserPretty,
isProduction && settings.isAnalyze && visualizer({
Expand All @@ -123,9 +119,6 @@ const terserPretty = terser({
keep_fnames: true,
ie8: false,
compress: {
// We need to hoist vars for process.env.NODE_ENV if-clauses for Metro:
hoist_vars: true,
hoist_funs: true,
pure_getters: true,
toplevel: true,
booleans_as_integers: false,
Expand All @@ -138,7 +131,10 @@ const terserPretty = terser({
conditionals: false,
join_vars: false
},
mangle: false,
mangle: {
module: true,
keep_fnames: true,
},
output: {
beautify: true,
braces: true,
Expand All @@ -156,6 +152,9 @@ const terserMinified = terser({
pure_getters: true,
passes: 10
},
mangle: {
module: true,
},
output: {
comments: false
}
Expand Down
Loading

0 comments on commit 25e6c5b

Please sign in to comment.