Skip to content

Commit 07b667c

Browse files
committed
Remove newly unreferenced VariableDeclarators
I've noticed that a common pattern used to assign propTypes--first define a variable for the propTypes, and then assign it to the component by reference--ends up deoptimizing this plugin, which leads to larger bundles in production than desired. The original hypothesis that minification would clean up these dead references works well for the simplest of propTypes, but unfortunately it doesn't hold up as soon as you have a function call in the propTypes variable (such as is the case for propTypes like `arrayOf`, `objectOf`, `shape`, etc.) because the minfier does not know that the function call will not have side-effects: ```js const propTypes = { foo: PropTypes.arrayOf(PropTypes.number), }; ``` I believe this should be solved by this plugin because we can make the assumption that function calls inside of propTypes have no side-effects. There seems to be a weird issue where the transform adds a semicolon when using in wrapped mode. For now, I decided to simply add the additional semicolons in the test fixtures, since they will be minified out anyway. I don't have much experience working with Babel plugins yet, so it is likely that I'm doing things in a weird or sub-optimal way here.
1 parent b2f03bb commit 07b667c

File tree

11 files changed

+308
-12
lines changed

11 files changed

+308
-12
lines changed

src/index.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,39 @@ function isReactClass(superClass, scope, globalOptions) {
5353
return answer
5454
}
5555

56+
function areSetsEqual(set1, set2) {
57+
if (set1.size !== set2.size) {
58+
return false
59+
}
60+
61+
return !Array.from(set1).some(item => !set2.has(item))
62+
}
63+
5664
export default function(api) {
5765
const { template, types, traverse } = api
5866

67+
const nestedIdentifiers = new Set()
68+
const removedPaths = new Set()
69+
const collectNestedIdentifiers = {
70+
Identifier(path) {
71+
if (path.parent.type === 'MemberExpression') {
72+
// foo.bar
73+
return
74+
}
75+
76+
if (
77+
path.parent.type === 'ObjectProperty' &&
78+
(path.parent.key === path.node || path.parent.shorthand)
79+
) {
80+
// { foo: 'bar' }
81+
// { foo }
82+
return
83+
}
84+
85+
nestedIdentifiers.add(path.node.name)
86+
},
87+
}
88+
5989
return {
6090
visitor: {
6191
Program(programPath, state) {
@@ -146,6 +176,8 @@ export default function(api) {
146176
})
147177

148178
if (parent) {
179+
path.traverse(collectNestedIdentifiers)
180+
removedPaths.add(path)
149181
remove(path, globalOptions, {
150182
type: 'createClass',
151183
})
@@ -160,6 +192,8 @@ export default function(api) {
160192
const pathClassDeclaration = scope.path
161193

162194
if (isReactClass(pathClassDeclaration.get('superClass'), scope, globalOptions)) {
195+
path.traverse(collectNestedIdentifiers)
196+
removedPaths.add(path)
163197
remove(path, globalOptions, {
164198
type: 'class static',
165199
pathClassDeclaration,
@@ -181,6 +215,8 @@ export default function(api) {
181215
const forceRemoval = isAnnotatedForRemoval(path.node.left)
182216

183217
if (forceRemoval) {
218+
path.traverse(collectNestedIdentifiers)
219+
removedPaths.add(path)
184220
remove(path, globalOptions, { type: 'assign' })
185221
return
186222
}
@@ -196,14 +232,61 @@ export default function(api) {
196232
const superClass = binding.path.get('superClass')
197233

198234
if (isReactClass(superClass, scope, globalOptions)) {
235+
path.traverse(collectNestedIdentifiers)
236+
removedPaths.add(path)
199237
remove(path, globalOptions, { type: 'assign' })
200238
}
201239
} else if (isStatelessComponent(binding.path)) {
240+
path.traverse(collectNestedIdentifiers)
241+
removedPaths.add(path)
202242
remove(path, globalOptions, { type: 'assign' })
203243
}
204244
},
205245
})
206246

247+
let skippedIdentifiers = 0
248+
const removeNewlyUnusedIdentifiers = {
249+
VariableDeclarator(path) {
250+
if (!nestedIdentifiers.has(path.node.id.name)) {
251+
return
252+
}
253+
254+
const { referencePaths } = path.scope.getBinding(path.node.id.name)
255+
256+
// Count the number of referencePaths that are not in the
257+
// removedPaths Set. We need to do this in order to support the wrap
258+
// option, which doesn't actually remove the references.
259+
const hasRemainingReferencePaths = referencePaths.some(referencePath => {
260+
const found = referencePath.find(p => removedPaths.has(p))
261+
return !found
262+
})
263+
264+
if (hasRemainingReferencePaths) {
265+
// There are still references to this identifier, so we need to
266+
// skip over it for now.
267+
skippedIdentifiers += 1
268+
return
269+
}
270+
271+
removedPaths.add(path)
272+
nestedIdentifiers.delete(path.node.id.name)
273+
path.get('init').traverse(collectNestedIdentifiers)
274+
remove(path, globalOptions, { type: 'declarator' })
275+
},
276+
}
277+
278+
let lastNestedIdentifiers = new Set()
279+
while (
280+
!areSetsEqual(nestedIdentifiers, lastNestedIdentifiers) &&
281+
nestedIdentifiers.size > 0 &&
282+
skippedIdentifiers < nestedIdentifiers.size
283+
) {
284+
lastNestedIdentifiers = new Set(nestedIdentifiers)
285+
skippedIdentifiers = 0
286+
programPath.scope.crawl()
287+
programPath.traverse(removeNewlyUnusedIdentifiers)
288+
}
289+
207290
if (globalOptions.removeImport) {
208291
if (globalOptions.mode === 'remove') {
209292
programPath.scope.crawl()

src/remove.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,24 @@ export default function remove(path, globalOptions, options) {
101101
path.node[visitedKey] = true
102102
break
103103

104+
case 'declarator':
105+
if (mode === 'unsafe-wrap') {
106+
path.replaceWith(
107+
unsafeWrapTemplate({
108+
NODE: path.node,
109+
})
110+
)
111+
} else {
112+
path.replaceWith(
113+
wrapTemplate({
114+
LEFT: path.node.id,
115+
RIGHT: path.node.init,
116+
})
117+
)
118+
}
119+
path.node[visitedKey] = true
120+
break
121+
104122
default:
105123
break
106124
}

test/fixtures/es-class-assign-property-variable/expected-remove-es5.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
"use strict";
22

3-
var propTypes = {
4-
foo: PropTypes.string
5-
};
6-
73
var Foo =
84
/*#__PURE__*/
95
function (_React$Component) {

test/fixtures/es-class-assign-property-variable/expected-remove-es6.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
const propTypes = {
2-
foo: PropTypes.string
3-
};
4-
51
class Foo extends React.Component {
62
render() {}
73

test/fixtures/es-class-assign-property-variable/expected-wrap-es5.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
"use strict";
22

3-
var propTypes = {
3+
var propTypes = process.env.NODE_ENV !== "production" ? {
44
foo: PropTypes.string
5-
};
5+
} : {};;
66

77
var Foo =
88
/*#__PURE__*/

test/fixtures/es-class-assign-property-variable/expected-wrap-es6.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
const propTypes = {
1+
const propTypes = process.env.NODE_ENV !== "production" ? {
22
foo: PropTypes.string
3-
};
3+
} : {};;
44

55
class Foo extends React.Component {
66
render() {}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
const propTypesBasic = {
2+
foo: PropTypes.string
3+
};
4+
5+
const FooBasic = () => (
6+
<div />
7+
);
8+
9+
FooBasic.propTypes = propTypesBasic;
10+
11+
const extraReference = {
12+
bar: PropTypes.string
13+
};
14+
15+
const propTypesWithExtraReference = Object.assign({}, extraReference, {
16+
foo: PropTypes.string
17+
});
18+
19+
const FooExtraReference = () => (
20+
<div />
21+
);
22+
23+
FooExtraReference.propTypes = propTypesWithExtraReference;
24+
25+
const propTypesWrapped = {
26+
foo: PropTypes.string
27+
};
28+
29+
const FooWrapped = () => (
30+
<div />
31+
);
32+
33+
FooWrapped.propTypes = someWrappingFunction(propTypesWrapped);
34+
35+
const propTypesReferenced = {
36+
foo: PropTypes.string
37+
};
38+
39+
const FooReferenced = () => (
40+
<div bar={propTypesReferenced} />
41+
);
42+
43+
FooReferenced.propTypes = propTypesReferenced;
44+
45+
export const propTypesExported = {
46+
foo: PropTypes.string
47+
};
48+
49+
const FooExported = () => (
50+
<div />
51+
);
52+
53+
FooExported.propTypes = propTypesExported;
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
"use strict";
2+
3+
Object.defineProperty(exports, "__esModule", {
4+
value: true
5+
});
6+
exports.propTypesExported = void 0;
7+
8+
var FooBasic = function FooBasic() {
9+
return React.createElement("div", null);
10+
};
11+
12+
var FooExtraReference = function FooExtraReference() {
13+
return React.createElement("div", null);
14+
};
15+
16+
var FooWrapped = function FooWrapped() {
17+
return React.createElement("div", null);
18+
};
19+
20+
var propTypesReferenced = {
21+
foo: PropTypes.string
22+
};
23+
24+
var FooReferenced = function FooReferenced() {
25+
return React.createElement("div", {
26+
bar: propTypesReferenced
27+
});
28+
};
29+
30+
var propTypesExported = {
31+
foo: PropTypes.string
32+
};
33+
exports.propTypesExported = propTypesExported;
34+
35+
var FooExported = function FooExported() {
36+
return React.createElement("div", null);
37+
};
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
const FooBasic = () => <div />;
2+
3+
const FooExtraReference = () => <div />;
4+
5+
const FooWrapped = () => <div />;
6+
7+
const propTypesReferenced = {
8+
foo: PropTypes.string
9+
};
10+
11+
const FooReferenced = () => <div bar={propTypesReferenced} />;
12+
13+
export const propTypesExported = {
14+
foo: PropTypes.string
15+
};
16+
17+
const FooExported = () => <div />;
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
"use strict";
2+
3+
Object.defineProperty(exports, "__esModule", {
4+
value: true
5+
});
6+
exports.propTypesExported = void 0;
7+
var propTypesBasic = process.env.NODE_ENV !== "production" ? {
8+
foo: PropTypes.string
9+
} : {};;
10+
11+
var FooBasic = function FooBasic() {
12+
return React.createElement("div", null);
13+
};
14+
15+
FooBasic.propTypes = process.env.NODE_ENV !== "production" ? propTypesBasic : {};
16+
var extraReference = process.env.NODE_ENV !== "production" ? {
17+
bar: PropTypes.string
18+
} : {};;
19+
var propTypesWithExtraReference = process.env.NODE_ENV !== "production" ? Object.assign({}, extraReference, {
20+
foo: PropTypes.string
21+
}) : {};;
22+
23+
var FooExtraReference = function FooExtraReference() {
24+
return React.createElement("div", null);
25+
};
26+
27+
FooExtraReference.propTypes = process.env.NODE_ENV !== "production" ? propTypesWithExtraReference : {};
28+
var propTypesWrapped = process.env.NODE_ENV !== "production" ? {
29+
foo: PropTypes.string
30+
} : {};;
31+
32+
var FooWrapped = function FooWrapped() {
33+
return React.createElement("div", null);
34+
};
35+
36+
FooWrapped.propTypes = process.env.NODE_ENV !== "production" ? someWrappingFunction(propTypesWrapped) : {};
37+
var propTypesReferenced = {
38+
foo: PropTypes.string
39+
};
40+
41+
var FooReferenced = function FooReferenced() {
42+
return React.createElement("div", {
43+
bar: propTypesReferenced
44+
});
45+
};
46+
47+
FooReferenced.propTypes = process.env.NODE_ENV !== "production" ? propTypesReferenced : {};
48+
var propTypesExported = {
49+
foo: PropTypes.string
50+
};
51+
exports.propTypesExported = propTypesExported;
52+
53+
var FooExported = function FooExported() {
54+
return React.createElement("div", null);
55+
};
56+
57+
FooExported.propTypes = process.env.NODE_ENV !== "production" ? propTypesExported : {};
58+

0 commit comments

Comments
 (0)