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

(chore) - remove closure-compiler #1570

Merged
merged 8 commits into from
Apr 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
---
Copy link
Member

Choose a reason for hiding this comment

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

This is just personal taste FWIW but I believe we shouldn't forcefully bump all builds, maybe? It'd be cool if there was a way to tell changesets to "ignore" one and only include it when a non-ignored change set was in a release too

'@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,
kitten marked this conversation as resolved.
Show resolved Hide resolved
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