Skip to content

Commit

Permalink
fix(shaker): reimplement enums support (fixes #848) (#853)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Anber
Anber committed Oct 8, 2021
1 parent 383886e commit 8f1d7cb
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 75 deletions.
94 changes: 31 additions & 63 deletions packages/babel/__tests__/depsGraph.test.ts
View file
Expand Up @@ -25,13 +25,13 @@ describe('VariableDeclaration', () => {

const deps = graph.getDependenciesByBinding('0:a');
expect(deps).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'NumericLiteral',
value: 42,
},
{
type: 'VariableDeclarator',
},
]);

expect(graph.findDependencies({ name: 'a' })).toContainEqual(deps[0]);
Expand All @@ -51,13 +51,13 @@ describe('scopes', () => {

const deps0 = graph.getDependenciesByBinding('0:a');
expect(deps0).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'NumericLiteral',
value: 42,
},
{
type: 'VariableDeclarator',
},
{
type: 'Identifier',
start: 4,
Expand All @@ -66,24 +66,24 @@ describe('scopes', () => {
type: 'Identifier',
start: 35,
},
{
type: 'AssignmentExpression',
},
{
type: 'NumericLiteral',
value: 21,
},
{
type: 'AssignmentExpression',
},
]);

const deps1 = graph.getDependenciesByBinding('1:a');
expect(deps1).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'StringLiteral',
value: '21',
},
{
type: 'VariableDeclarator',
},
]);

expect(graph.findDependencies({ name: 'a' })).toHaveLength(8);
Expand All @@ -97,14 +97,14 @@ describe('scopes', () => {
const aDeps = graph.getDependenciesByBinding('0:a');
expect(aDeps).toMatchObject([
{
type: 'VariableDeclarator',
type: 'ArrowFunctionExpression',
},
{
type: 'ArrowFunctionExpression',
type: 'VariableDeclarator',
},
]);

expect(graph.getDependenciesByBinding('1:arg1')).toHaveLength(3);
expect(graph.getDependenciesByBinding('1:arg1')).toHaveLength(1);
expect(graph.getDependentsByBinding('1:arg1')).toMatchObject([
{
// arg1 in the binary expression
Expand All @@ -129,33 +129,19 @@ describe('scopes', () => {
]);

expect(graph.getDependenciesByBinding('1:arg2')).toMatchObject([
{
type: 'ArrowFunctionExpression',
},
{
type: 'Identifier',
name: 'arg2',
start: 17,
},
{
type: 'BinaryExpression',
start: 32,
},
]);

expect(graph.getDependenciesByBinding('1:arg3')).toMatchObject([
{
type: 'ArrowFunctionExpression',
},
{
type: 'Identifier',
name: 'arg3',
start: 23,
},
{
type: 'BinaryExpression',
start: 32,
},
]);
});
});
Expand All @@ -169,13 +155,13 @@ describe('AssignmentExpression', () => {

const deps = graph.getDependenciesByBinding('0:a');
expect(deps).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'NumericLiteral',
value: 42,
},
{
type: 'VariableDeclarator',
},
{
type: 'Identifier',
name: 'a',
Expand All @@ -186,13 +172,13 @@ describe('AssignmentExpression', () => {
name: 'a',
start: 12,
},
{
type: 'AssignmentExpression',
},
{
type: 'NumericLiteral',
value: 24,
},
{
type: 'AssignmentExpression',
},
]);

expect(graph.findDependents({ value: 42 })).toHaveLength(1);
Expand All @@ -206,13 +192,13 @@ describe('AssignmentExpression', () => {
`;

expect(graph.getDependenciesByBinding('0:a')).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'ObjectExpression',
properties: [],
},
{
type: 'VariableDeclarator',
},
{
type: 'Identifier',
name: 'a',
Expand Down Expand Up @@ -256,12 +242,6 @@ it('SequenceExpression', () => {
{
type: 'ArrowFunctionExpression',
},
{
id: {
name: 'color2',
},
type: 'VariableDeclarator',
},
]);

const fnDeps = graph.findDependencies({
Expand All @@ -280,20 +260,17 @@ it('SequenceExpression', () => {
name: 'local',
type: 'Identifier',
},
{
type: 'SequenceExpression',
},
]);

const localDeps = graph.getDependenciesByBinding('0:local');
expect(localDeps).toMatchObject([
{
type: 'VariableDeclarator',
},
{
type: 'StringLiteral',
value: '',
},
{
type: 'VariableDeclarator',
},
{
type: 'Identifier',
name: 'local',
Expand All @@ -304,26 +281,23 @@ it('SequenceExpression', () => {
name: 'local',
start: 61,
},
{
type: 'AssignmentExpression',
},
{
type: 'Identifier',
name: 'color1',
},
{
type: 'AssignmentExpression',
},
{
type: 'Identifier',
name: 'local',
start: 27,
},
{
type: 'ArrowFunctionExpression',
},
]);

const bool = { type: 'BooleanLiteral' };
expect(graph.findDependents(bool)).toHaveLength(0);
expect(graph.findDependencies(bool)).toHaveLength(1);
expect(graph.findDependencies(bool)).toHaveLength(0);
});

it('MemberExpression', () => {
Expand All @@ -346,11 +320,5 @@ it('MemberExpression', () => {
type: 'Identifier',
name: 'key',
},
{
type: 'VariableDeclarator',
id: {
name: 'blue',
},
},
]);
});
4 changes: 2 additions & 2 deletions packages/shaker/src/graphBuilder.ts
View file
Expand Up @@ -194,8 +194,8 @@ class GraphBuilder extends GraphBuilderState {
this.baseVisit(node);
}

if (parent && action !== 'ignore') {
// Node always depends on its parent
if (parent && action !== 'ignore' && t.isStatement(node)) {
// Statement always depends on its parent
this.graph.addEdge(node, parent);
}

Expand Down
76 changes: 66 additions & 10 deletions packages/shaker/src/langs/core.ts
View file
Expand Up @@ -213,7 +213,60 @@ function getAffectedNodes(node: Node, state: GraphBuilderState): Node[] {
return [];
}

/*
* In some cases (such as enums) babel uses CallExpression for object initializations
* (function (Colors) {
* Colors["BLUE"] = "#27509A";
* })(Colors || (Colors = {}));
*/
function isLazyInit(
statement: ExpressionStatement
): statement is ExpressionStatement & {
expression: {
arguments: [{ right: AssignmentExpression }];
};
} {
const { expression } = statement;
if (!t.isCallExpression(expression) || expression.arguments.length !== 1) {
return false;
}

const [arg] = expression.arguments;
if (!t.isLogicalExpression(arg) || arg.operator !== '||') {
return false;
}

const { left, right } = arg;
return t.isIdentifier(left) && t.isAssignmentExpression(right);
}

export const visitors: Visitors = {
/*
* ExpressionStatement
* This is one of the rare cases when a child defines a dependency on a parent.
* Suppose we have a code like this:
* const fn = () => {
* let a = 2;
* a *= 2;
* return a;
* };
*
* `a *= 2` here is an ExpressionStatement node which contains an expression AssignmentExpression `a *= 2`.
* The result of AssignmentExpression here depends on the fact of ExpressionStatement execution,
* that's why we need to mark the statement as a dependency of the expression.
* If we don't mark it, it will be cut as a useless statement.
*/
ExpressionStatement(this: GraphBuilderState, node: ExpressionStatement) {
this.baseVisit(node);

this.graph.addEdge(node, node.expression);
this.graph.addEdge(node.expression, node);

if (isLazyInit(node)) {
this.graph.addEdge(node.expression.arguments[0].right, node);
}
},

/*
* FunctionDeclaration | FunctionExpression | ObjectMethod | ArrowFunctionExpression | ClassMethod | ClassPrivateMethod;
* Functions can be either a statement or an expression.
Expand All @@ -232,6 +285,11 @@ export const visitors: Visitors = {
this.graph.addEdge(node, node.body);

node.params.forEach((param) => this.graph.addEdge(node.body, param));
if (t.isFunctionDeclaration(node) && node.id) {
// `id` is an identifier which depends on the function declaration
this.graph.addEdge(node.id, node);
}

if (
t.isFunctionExpression(node) &&
node.id !== null &&
Expand All @@ -246,16 +304,6 @@ export const visitors: Visitors = {
}
},

/*
* ExpressionStatement
*/
ExpressionStatement(this: GraphBuilderState, node: ExpressionStatement) {
this.baseVisit(node);

this.graph.addEdge(node, node.expression);
this.graph.addEdge(node.expression, node);
},

/*
* BlockStatement | Program
* The same situation as in ExpressionStatement: if one of the expressions is required, the block itself is also required.
Expand Down Expand Up @@ -430,6 +478,7 @@ export const visitors: Visitors = {
*/
MemberExpression(this: GraphBuilderState, node: MemberExpression) {
this.baseVisit(node);
this.graph.addEdge(node.object, node);

if (
isIdentifier(node.object, 'exports') &&
Expand Down Expand Up @@ -494,6 +543,9 @@ export const visitors: Visitors = {

// The left part of an assignment depends on the right part.
this.graph.addEdge(node.left, node.right);

// At the same time, the left part doesn't make any sense without the whole expression.
this.graph.addEdge(node.left, node);
},

/*
Expand Down Expand Up @@ -521,6 +573,10 @@ export const visitors: Visitors = {
this.graph.addEdge(node.id, node.init);
}

// If we want to evaluate the value of a declared identifier,
// we need to evaluate the whole expression.
this.graph.addEdge(node.id, node);

// If a statement is required itself, an id is also required
this.graph.addEdge(node, node.id);
},
Expand Down

0 comments on commit 8f1d7cb

Please sign in to comment.