Skip to content
This repository was archived by the owner on Aug 4, 2021. It is now read-only.

Commit c8c2548

Browse files
Rich-HarrisRich-Harris
committed
prefer existing names for dependencies, over require$$x (#176)
1 parent b8c0c77 commit c8c2548

4 files changed

Lines changed: 151 additions & 57 deletions

File tree

‎src/ast-utils.js‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,39 @@ export function flatten ( node ) {
3131
return { name, keypath: parts.join( '.' ) };
3232
}
3333

34+
export function extractNames ( node ) {
35+
const names = [];
36+
extractors[ node.type ]( names, node );
37+
return names;
38+
}
39+
40+
const extractors = {
41+
Identifier ( names, node ) {
42+
names.push( node.name );
43+
},
44+
45+
ObjectPattern ( names, node ) {
46+
node.properties.forEach( prop => {
47+
extractors[ prop.value.type ]( names, prop.value );
48+
});
49+
},
50+
51+
ArrayPattern ( names, node ) {
52+
node.elements.forEach( element => {
53+
if ( element ) extractors[ element.type ]( names, element );
54+
});
55+
},
56+
57+
RestElement ( names, node ) {
58+
extractors[ node.argument.type ]( names, node.argument );
59+
},
60+
61+
AssignmentPattern ( names, node ) {
62+
extractors[ node.left.type ]( names, node.left );
63+
}
64+
};
65+
66+
3467
export function isTruthy ( node ) {
3568
if ( node.type === 'Literal' ) return !!node.value;
3669
if ( node.type === 'ParenthesizedExpression' ) return isTruthy( node.expression );

‎src/transform.js‎

Lines changed: 114 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import acorn from 'acorn';
22
import { walk } from 'estree-walker';
33
import MagicString from 'magic-string';
44
import { attachScopes, makeLegalIdentifier } from 'rollup-pluginutils';
5-
import { flatten, isReference, isTruthy, isFalsy } from './ast-utils.js';
5+
import { extractNames, flatten, isReference, isTruthy, isFalsy } from './ast-utils.js';
66
import { PREFIX, HELPERS_ID } from './helpers.js';
77
import { getName } from './utils.js';
88

@@ -73,6 +73,45 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
7373
// TODO handle transpiled modules
7474
let shouldWrap = /__esModule/.test( code );
7575

76+
function isRequireStatement ( node ) {
77+
if ( !node ) return;
78+
if ( node.type !== 'CallExpression' ) return;
79+
if ( node.callee.name !== 'require' || scope.contains( 'require' ) ) return;
80+
if ( node.arguments.length !== 1 || node.arguments[0].type !== 'Literal' ) return; // TODO handle these weird cases?
81+
if ( ignoreRequire( node.arguments[0].value ) ) return;
82+
83+
return true;
84+
}
85+
86+
function getRequired ( node, name ) {
87+
const source = node.arguments[0].value;
88+
89+
const existing = required[ source ];
90+
if ( existing === undefined ) {
91+
sources.push( source );
92+
93+
if ( !name ) name = `require$$${uid++}`;
94+
required[ source ] = { source, name, importsDefault: false };
95+
}
96+
97+
return required[ source ];
98+
}
99+
100+
// do a first pass, see which names are assigned to. This is necessary to prevent
101+
// illegally replacing `var foo = require('foo')` with `import foo from 'foo'`,
102+
// where `foo` is later reassigned. (This happens in the wild. CommonJS, sigh)
103+
const assignedTo = new Set();
104+
walk( ast, {
105+
enter ( node ) {
106+
if ( node.type !== 'AssignmentExpression' ) return;
107+
if ( node.left.type === 'MemberExpression' ) return;
108+
109+
extractNames( node.left ).forEach( name => {
110+
assignedTo.add( name );
111+
});
112+
}
113+
});
114+
76115
walk( ast, {
77116
enter ( node, parent ) {
78117
if ( sourceMap ) {
@@ -93,6 +132,13 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
93132
if ( node.scope ) scope = node.scope;
94133
if ( /^Function/.test( node.type ) ) lexicalDepth += 1;
95134

135+
// rewrite `this` as `commonjsHelpers.commonjsGlobal`
136+
if ( node.type === 'ThisExpression' && lexicalDepth === 0 ) {
137+
uses.global = true;
138+
if ( !ignoreGlobal ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
139+
return;
140+
}
141+
96142
// rewrite `typeof module`, `typeof module.exports` and `typeof exports` (https://github.com/rollup/rollup-plugin-commonjs/issues/151)
97143
if ( node.type === 'UnaryExpression' && node.operator === 'typeof' ) {
98144
const flattened = flatten( node.argument );
@@ -105,6 +151,38 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
105151
}
106152
}
107153

154+
// rewrite `require` (if not already handled) `global` and `define`, and handle free references to
155+
// `module` and `exports` as these mean we need to wrap the module in commonjsHelpers.createCommonjsModule
156+
if ( node.type === 'Identifier' ) {
157+
if ( isReference( node, parent ) && !scope.contains( node.name ) ) {
158+
if ( node.name in uses ) {
159+
if ( node.name === 'require' ) {
160+
if ( allowDynamicRequire ) return;
161+
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, true );
162+
}
163+
164+
uses[ node.name ] = true;
165+
if ( node.name === 'global' && !ignoreGlobal ) {
166+
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
167+
}
168+
169+
// if module or exports are used outside the context of an assignment
170+
// expression, we need to wrap the module
171+
if ( node.name === 'module' || node.name === 'exports' ) {
172+
shouldWrap = true;
173+
}
174+
}
175+
176+
if ( node.name === 'define' ) {
177+
magicString.overwrite( node.start, node.end, 'undefined', true );
178+
}
179+
180+
globals.add( node.name );
181+
}
182+
183+
return;
184+
}
185+
108186
// Is this an assignment to exports or module.exports?
109187
if ( node.type === 'AssignmentExpression' ) {
110188
if ( node.left.type !== 'MemberExpression' ) return;
@@ -137,68 +215,32 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
137215
return;
138216
}
139217

140-
if ( node.type === 'Identifier' ) {
141-
if ( isReference( node, parent ) && !scope.contains( node.name ) ) {
142-
if ( node.name in uses ) {
143-
if ( node.name === 'require' ) {
144-
if ( allowDynamicRequire ) return;
145-
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, true );
146-
}
147-
148-
uses[ node.name ] = true;
149-
if ( node.name === 'global' && !ignoreGlobal ) {
150-
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
151-
}
218+
// if this is `var x = require('x')`, we can do `import x from 'x'`
219+
if ( node.type === 'VariableDeclarator' && node.id.type === 'Identifier' && isRequireStatement( node.init ) ) {
220+
// for now, only do this for top-level requires. maybe fix this in future
221+
if ( scope.parent ) return;
152222

153-
// if module or exports are used outside the context of an assignment
154-
// expression, we need to wrap the module
155-
if ( node.name === 'module' || node.name === 'exports' ) {
156-
shouldWrap = true;
157-
}
158-
}
223+
// edge case — CJS allows you to assign to imports. ES doesn't
224+
if ( assignedTo.has( node.id.name ) ) return;
159225

160-
if ( node.name === 'define' ) {
161-
magicString.overwrite( node.start, node.end, 'undefined', true );
162-
}
226+
const r = getRequired( node.init, node.id.name );
227+
r.importsDefault = true;
163228

164-
globals.add( node.name );
229+
if ( r.name === node.id.name ) {
230+
node._shouldRemove = true;
165231
}
166-
167-
return;
168-
}
169-
170-
if ( node.type === 'ThisExpression' && lexicalDepth === 0 ) {
171-
uses.global = true;
172-
if ( !ignoreGlobal ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
173-
return;
174232
}
175233

176-
if ( node.type !== 'CallExpression' ) return;
177-
if ( node.callee.name !== 'require' || scope.contains( 'require' ) ) return;
178-
if ( node.arguments.length !== 1 || node.arguments[0].type !== 'Literal' ) return; // TODO handle these weird cases?
179-
if ( ignoreRequire( node.arguments[0].value ) ) return;
180-
181-
const source = node.arguments[0].value;
234+
if ( !isRequireStatement( node ) ) return;
182235

183-
const existing = required[ source ];
184-
if ( existing === undefined ) {
185-
sources.push( source );
186-
}
187-
let name;
236+
const r = getRequired( node );
188237

189-
if ( !existing ) {
190-
name = `require$$${uid++}`;
191-
required[ source ] = { source, name, importsDefault: false };
192-
} else {
193-
name = required[ source ].name;
194-
}
195-
196-
if ( parent.type !== 'ExpressionStatement' ) {
197-
required[ source ].importsDefault = true;
198-
magicString.overwrite( node.start, node.end, name );
199-
} else {
238+
if ( parent.type === 'ExpressionStatement' ) {
200239
// is a bare import, e.g. `require('foo');`
201240
magicString.remove( parent.start, parent.end );
241+
} else {
242+
r.importsDefault = true;
243+
magicString.overwrite( node.start, node.end, r.name );
202244
}
203245

204246
node.callee._skip = true;
@@ -208,6 +250,25 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
208250
programDepth -= 1;
209251
if ( node.scope ) scope = scope.parent;
210252
if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;
253+
254+
if ( node.type === 'VariableDeclaration' ) {
255+
let keepDeclaration = false;
256+
257+
for ( let i = 0; i < node.declarations.length; i += 1 ) {
258+
const declarator = node.declarations[i];
259+
const next = node.declarations[ i + 1 ];
260+
261+
if ( declarator._shouldRemove ) {
262+
magicString.remove( declarator.start, next ? next.start : declarator.end );
263+
} else {
264+
keepDeclaration = true;
265+
}
266+
}
267+
268+
if ( !keepDeclaration ) {
269+
magicString.remove( node.start, node.end );
270+
}
271+
}
211272
}
212273
});
213274

‎test/form/ignore-ids-function/output.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import 'bar';
2-
import require$$0 from 'commonjs-proxy:bar';
2+
import bar from 'commonjs-proxy:bar';
33

44
var foo = require( 'foo' );
5-
var bar = require$$0;
5+
66

77
var input = {
88

‎test/form/ignore-ids/output.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import 'bar';
2-
import require$$0 from 'commonjs-proxy:bar';
2+
import bar from 'commonjs-proxy:bar';
33

44
var foo = require( 'foo' );
5-
var bar = require$$0;
5+
66

77
var input = {
88

0 commit comments

Comments
 (0)