Skip to content

Commit 3c78036

Browse files
authored
Remove unused MemberExpression root identifiers (#158)
In our codebase, I noticed that we had the following pattern that was not getting properly cleaned up: ```js const shapePropType = PropTypes.shape({ foo: PropTypes.string, }); const ComponentA = () => <div />; ComponentA.propTypes = { foo: shapePropType.isRequired, }; ``` The notable thing here is that inside the propTypes assignment, a MemberExpression is used instead of just an identifier. However, in our visitor for collecting nested identifiers, we special-cased Identifiers that have a MemberExpression parent to not be collected. This was to prevent the `bar` portion of `foo.bar` from being collected. However, in this case, we actually want the `foo` part of `foo.bar` to be collected for possible additional cleanup, so we need to add a little more logic to make this happen. To solve this, I added a function that takes a path and traverses up the tree until it finds the first non-MemberExpression, and then traverses down the left side of that tree until it finds the root identifier. This should be the `foo` in `foo.bar.baz`, for instance. It seems likely that there is a better more built-in Babel way to do this, but I was unable to find anything to point me in that direction so I rolled my own.
1 parent 5fa6824 commit 3c78036

File tree

7 files changed

+200
-2
lines changed

7 files changed

+200
-2
lines changed

src/index.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,35 @@ function areSetsEqual(set1, set2) {
6565
return !Array.from(set1).some(item => !set2.has(item))
6666
}
6767

68+
function memberExpressionRootIdentifier(path) {
69+
// Traverse up to the parent before the topmost member expression, and then
70+
// traverse back down to find the topmost identifier. It seems like there
71+
// might be a better way to do this.
72+
const parent = path.findParent(p => !p.isMemberExpression())
73+
const { type } = parent.node
74+
75+
let memberExpression
76+
if (type === 'ObjectProperty') {
77+
// The topmost MemberExpression's parent is an object property, so the
78+
// topmost MemberExpression should be the value.
79+
memberExpression = parent.get('value')
80+
}
81+
82+
if (!memberExpression) {
83+
// This case is currently unhandled by this plugin.
84+
return null
85+
}
86+
87+
// We have a topmost MemberExpression now, so we want to traverse down the
88+
// left half untli we no longer see MemberExpressions. This node will give us
89+
// our leftmost identifier.
90+
while (memberExpression.node.object.type === 'MemberExpression') {
91+
memberExpression = memberExpression.get('object')
92+
}
93+
94+
return memberExpression.get('object')
95+
}
96+
6897
export default function(api) {
6998
const { template, types, traverse } = api
7099

@@ -74,6 +103,12 @@ export default function(api) {
74103
Identifier(path) {
75104
if (path.parent.type === 'MemberExpression') {
76105
// foo.bar
106+
107+
const root = memberExpressionRootIdentifier(path)
108+
if (root) {
109+
nestedIdentifiers.add(root.node.name)
110+
}
111+
77112
return
78113
}
79114

@@ -188,6 +223,7 @@ export default function(api) {
188223
}
189224
},
190225
},
226+
191227
// Here to support stage-1 transform-class-properties.
192228
ClassProperty(path) {
193229
const { node, scope } = path
@@ -205,6 +241,7 @@ export default function(api) {
205241
}
206242
}
207243
},
244+
208245
AssignmentExpression(path) {
209246
const { node, scope } = path
210247

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
11
var createReactClass = require('create-react-class');
22

3-
var PropTypes = require('prop-types');
4-
53
createReactClass({});
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
const shapePropType = PropTypes.shape({
2+
foo: PropTypes.string,
3+
});
4+
5+
const ComponentA = () => <div />;
6+
ComponentA.propTypes = {
7+
foo: shapePropType.isRequired,
8+
};
9+
10+
const somePropTypes = {
11+
foo: PropTypes.string,
12+
bar: PropTypes.number,
13+
};
14+
15+
const ComponentB = () => <div />;
16+
ComponentB.propTypes = {
17+
foo: somePropTypes.foo.isRequired,
18+
};
19+
20+
const somePropTypesC = {
21+
foo: PropTypes.string,
22+
bar: PropTypes.number,
23+
};
24+
25+
const ComponentC = () => <div />;
26+
ComponentC.propTypes = {
27+
foo: somePropTypesC['foo'].isRequired,
28+
};
29+
30+
const somePropTypesD = {
31+
foo: PropTypes.string,
32+
bar: PropTypes.number,
33+
};
34+
35+
const ComponentD = () => <div />;
36+
const foo = { bar: 'foo' };
37+
ComponentD.propTypes = {
38+
[foo.bar]: somePropTypesD['foo'].isRequired,
39+
};
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
"use strict";
2+
3+
var ComponentA = function ComponentA() {
4+
return React.createElement("div", null);
5+
};
6+
7+
var ComponentB = function ComponentB() {
8+
return React.createElement("div", null);
9+
};
10+
11+
var ComponentC = function ComponentC() {
12+
return React.createElement("div", null);
13+
};
14+
15+
var ComponentD = function ComponentD() {
16+
return React.createElement("div", null);
17+
};
18+
19+
var foo = {
20+
bar: 'foo'
21+
};
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
const ComponentA = () => <div />;
2+
3+
const ComponentB = () => <div />;
4+
5+
const ComponentC = () => <div />;
6+
7+
const ComponentD = () => <div />;
8+
9+
const foo = {
10+
bar: 'foo'
11+
};
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
"use strict";
2+
3+
var shapePropType = process.env.NODE_ENV !== "production" ? PropTypes.shape({
4+
foo: PropTypes.string
5+
}) : {};;
6+
7+
var ComponentA = function ComponentA() {
8+
return React.createElement("div", null);
9+
};
10+
11+
ComponentA.propTypes = process.env.NODE_ENV !== "production" ? {
12+
foo: shapePropType.isRequired
13+
} : {};
14+
var somePropTypes = process.env.NODE_ENV !== "production" ? {
15+
foo: PropTypes.string,
16+
bar: PropTypes.number
17+
} : {};;
18+
19+
var ComponentB = function ComponentB() {
20+
return React.createElement("div", null);
21+
};
22+
23+
ComponentB.propTypes = process.env.NODE_ENV !== "production" ? {
24+
foo: somePropTypes.foo.isRequired
25+
} : {};
26+
var somePropTypesC = process.env.NODE_ENV !== "production" ? {
27+
foo: PropTypes.string,
28+
bar: PropTypes.number
29+
} : {};;
30+
31+
var ComponentC = function ComponentC() {
32+
return React.createElement("div", null);
33+
};
34+
35+
ComponentC.propTypes = process.env.NODE_ENV !== "production" ? {
36+
foo: somePropTypesC['foo'].isRequired
37+
} : {};
38+
var somePropTypesD = process.env.NODE_ENV !== "production" ? {
39+
foo: PropTypes.string,
40+
bar: PropTypes.number
41+
} : {};;
42+
43+
var ComponentD = function ComponentD() {
44+
return React.createElement("div", null);
45+
};
46+
47+
var foo = {
48+
bar: 'foo'
49+
};
50+
ComponentD.propTypes = process.env.NODE_ENV !== "production" ? babelHelpers.defineProperty({}, foo.bar, somePropTypesD['foo'].isRequired) : {};
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
const shapePropType = process.env.NODE_ENV !== "production" ? PropTypes.shape({
2+
foo: PropTypes.string
3+
}) : {};;
4+
5+
const ComponentA = () => <div />;
6+
7+
ComponentA.propTypes = process.env.NODE_ENV !== "production" ? {
8+
foo: shapePropType.isRequired
9+
} : {};
10+
const somePropTypes = process.env.NODE_ENV !== "production" ? {
11+
foo: PropTypes.string,
12+
bar: PropTypes.number
13+
} : {};;
14+
15+
const ComponentB = () => <div />;
16+
17+
ComponentB.propTypes = process.env.NODE_ENV !== "production" ? {
18+
foo: somePropTypes.foo.isRequired
19+
} : {};
20+
const somePropTypesC = process.env.NODE_ENV !== "production" ? {
21+
foo: PropTypes.string,
22+
bar: PropTypes.number
23+
} : {};;
24+
25+
const ComponentC = () => <div />;
26+
27+
ComponentC.propTypes = process.env.NODE_ENV !== "production" ? {
28+
foo: somePropTypesC['foo'].isRequired
29+
} : {};
30+
const somePropTypesD = process.env.NODE_ENV !== "production" ? {
31+
foo: PropTypes.string,
32+
bar: PropTypes.number
33+
} : {};;
34+
35+
const ComponentD = () => <div />;
36+
37+
const foo = {
38+
bar: 'foo'
39+
};
40+
ComponentD.propTypes = process.env.NODE_ENV !== "production" ? {
41+
[foo.bar]: somePropTypesD['foo'].isRequired
42+
} : {};

0 commit comments

Comments
 (0)