Skip to content

Commit 8f1a79a

Browse files
authored
fix: handle temporal deadzone for local variables shadowing parent function (#91)
1 parent 193a486 commit 8f1a79a

File tree

2 files changed

+105
-3
lines changed

2 files changed

+105
-3
lines changed

src/babel/__tests__/unit.test.js

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,95 @@ describe('babel plugin', () => {
103103
`);
104104
});
105105

106+
describe('shadowed variables', () => {
107+
it('simple case: should remove', () => {
108+
const input = `
109+
import LinkifyIt from 'linkify-it';
110+
const linkify = (state) => {
111+
// cannot refer to the function location ^ as there is a local variable shadowing it
112+
const linkify = new LinkifyIt();
113+
}
114+
`;
115+
expect(babel(input)).toMatchInlineSnapshot(`
116+
"import { di as _di } from "react-magnetic-di";
117+
import LinkifyIt from 'linkify-it';
118+
const linkify = state => {
119+
const [_LinkifyIt] = _di([LinkifyIt], null);
120+
// cannot refer to the function location ^ as there is a local variable shadowing it
121+
const linkify = new _LinkifyIt();
122+
};"
123+
`);
124+
});
125+
126+
it('nested function case: should keep', () => {
127+
const input = `
128+
import LinkifyIt from 'linkify-it';
129+
const linkify = (state) => {
130+
useEffect(() => {
131+
const linkify = new LinkifyIt();
132+
});
133+
}
134+
`;
135+
expect(babel(input)).toMatchInlineSnapshot(`
136+
"import { di as _di } from "react-magnetic-di";
137+
import LinkifyIt from 'linkify-it';
138+
const linkify = state => {
139+
const [_LinkifyIt] = _di([LinkifyIt], linkify);
140+
useEffect(() => {
141+
const [_LinkifyIt2] = _di([_LinkifyIt], null);
142+
const linkify = new _LinkifyIt2();
143+
});
144+
};"
145+
`);
146+
});
147+
148+
it('block case: should keep', () => {
149+
const input = `
150+
import LinkifyIt from 'linkify-it';
151+
const linkify = (state) => {
152+
if (ff('xx')) {
153+
const linkify = new LinkifyIt();
154+
}
155+
}
156+
`;
157+
expect(babel(input)).toMatchInlineSnapshot(`
158+
"import { di as _di } from "react-magnetic-di";
159+
import LinkifyIt from 'linkify-it';
160+
const linkify = state => {
161+
const [_LinkifyIt] = _di([LinkifyIt], linkify);
162+
if (ff('xx')) {
163+
const linkify = new _LinkifyIt();
164+
}
165+
};"
166+
`);
167+
});
168+
169+
it('mixed case: should keep', () => {
170+
const input = `
171+
import LinkifyIt from 'linkify-it';
172+
import LinkifyThat from 'linkify-it';
173+
const linkify = (state) => {
174+
const linkify = new LinkifyIt();
175+
if (ff('xx')) {
176+
const linkify = new LinkifyThat();
177+
}
178+
}
179+
`;
180+
expect(babel(input)).toMatchInlineSnapshot(`
181+
"import { di as _di } from "react-magnetic-di";
182+
import LinkifyIt from 'linkify-it';
183+
import LinkifyThat from 'linkify-it';
184+
const linkify = state => {
185+
const [_LinkifyIt, _LinkifyThat] = _di([LinkifyIt, LinkifyThat], null);
186+
const linkify = new _LinkifyIt();
187+
if (ff('xx')) {
188+
const linkify = new _LinkifyThat();
189+
}
190+
};"
191+
`);
192+
});
193+
});
194+
106195
it('should work and maintain location if manually declared', () => {
107196
const input = `
108197
import React from 'react';

src/babel/processor-di.js

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,32 @@ function processReference(t, path, locationValue, state) {
44
const self = getComponentDeclaration(t, path.scope);
55
const bodyPath = path.get('body');
66

7+
let shadowsOwnName = false;
8+
if(self) {
9+
const selfShadow = bodyPath.scope.getBinding(self.name);
10+
if(selfShadow && selfShadow.scope===bodyPath.scope && selfShadow.path.node.id!==self){
11+
shadowsOwnName = true;
12+
}
13+
}
714
// Build list of dependencies
815
// combining used imports/exports in this function block
916
// with existing di expression (if any)
1017
const depNames = [];
18+
1119
Array.from(locationValue.dependencyRefs).forEach((n) => {
1220
const name = n.node?.name;
1321
// quick check that the path is not detached
1422
if (!name || !n.parentPath) return;
1523
// Some babel plugins might rename imports (eg emotion) and references break
1624
// For now we skip, but ideally we would refresh the reference
1725
if (!bodyPath.scope.getBinding(name)) return;
18-
// Ensure we do not duplicate and di() self name
19-
if (depNames.includes(name) || name === self?.name) return;
26+
// Ensure we do not di() self name
27+
if (name === self?.name) {
28+
shadowsOwnName = true;
29+
return;
30+
}
31+
// Ensure we do not duplicate
32+
if (depNames.includes(name)) return;
2033

2134
depNames.push(name);
2235
});
@@ -37,7 +50,7 @@ function processReference(t, path, locationValue, state) {
3750
t.arrayPattern(elements),
3851
t.callExpression(t.identifier(state.diIdentifier.name), [
3952
t.arrayExpression(args),
40-
self ? t.identifier(self.name) : t.nullLiteral(),
53+
self && !shadowsOwnName ? t.identifier(self.name) : t.nullLiteral(),
4154
])
4255
),
4356
]);

0 commit comments

Comments
 (0)