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

Commit

Permalink
prefer existing names for dependencies, over require$$x (#176)
Browse files Browse the repository at this point in the history
  • Loading branch information
@Rich-Harris
Rich-Harris committed Mar 10, 2017
1 parent b8c0c77 commit c8c2548
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 57 deletions.
33 changes: 33 additions & 0 deletions src/ast-utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,39 @@ export function flatten ( node ) {
return { name, keypath: parts.join( '.' ) };
}

export function extractNames ( node ) {
const names = [];
extractors[ node.type ]( names, node );
return names;
}

const extractors = {
Identifier ( names, node ) {
names.push( node.name );
},

ObjectPattern ( names, node ) {
node.properties.forEach( prop => {
extractors[ prop.value.type ]( names, prop.value );
});
},

ArrayPattern ( names, node ) {
node.elements.forEach( element => {
if ( element ) extractors[ element.type ]( names, element );
});
},

RestElement ( names, node ) {
extractors[ node.argument.type ]( names, node.argument );
},

AssignmentPattern ( names, node ) {
extractors[ node.left.type ]( names, node.left );
}
};


export function isTruthy ( node ) {
if ( node.type === 'Literal' ) return !!node.value;
if ( node.type === 'ParenthesizedExpression' ) return isTruthy( node.expression );
Expand Down
167 changes: 114 additions & 53 deletions src/transform.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import acorn from 'acorn';
import { walk } from 'estree-walker';
import MagicString from 'magic-string';
import { attachScopes, makeLegalIdentifier } from 'rollup-pluginutils';
import { flatten, isReference, isTruthy, isFalsy } from './ast-utils.js';
import { extractNames, flatten, isReference, isTruthy, isFalsy } from './ast-utils.js';
import { PREFIX, HELPERS_ID } from './helpers.js';
import { getName } from './utils.js';

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

function isRequireStatement ( node ) {
if ( !node ) return;
if ( node.type !== 'CallExpression' ) return;
if ( node.callee.name !== 'require' || scope.contains( 'require' ) ) return;
if ( node.arguments.length !== 1 || node.arguments[0].type !== 'Literal' ) return; // TODO handle these weird cases?
if ( ignoreRequire( node.arguments[0].value ) ) return;

return true;
}

function getRequired ( node, name ) {
const source = node.arguments[0].value;

const existing = required[ source ];
if ( existing === undefined ) {
sources.push( source );

if ( !name ) name = `require$$${uid++}`;
required[ source ] = { source, name, importsDefault: false };
}

return required[ source ];
}

// do a first pass, see which names are assigned to. This is necessary to prevent
// illegally replacing `var foo = require('foo')` with `import foo from 'foo'`,
// where `foo` is later reassigned. (This happens in the wild. CommonJS, sigh)
const assignedTo = new Set();
walk( ast, {
enter ( node ) {
if ( node.type !== 'AssignmentExpression' ) return;
if ( node.left.type === 'MemberExpression' ) return;

extractNames( node.left ).forEach( name => {
assignedTo.add( name );
});
}
});

walk( ast, {
enter ( node, parent ) {
if ( sourceMap ) {
Expand All @@ -93,6 +132,13 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
if ( node.scope ) scope = node.scope;
if ( /^Function/.test( node.type ) ) lexicalDepth += 1;

// rewrite `this` as `commonjsHelpers.commonjsGlobal`
if ( node.type === 'ThisExpression' && lexicalDepth === 0 ) {
uses.global = true;
if ( !ignoreGlobal ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
return;
}

// rewrite `typeof module`, `typeof module.exports` and `typeof exports` (https://github.com/rollup/rollup-plugin-commonjs/issues/151)
if ( node.type === 'UnaryExpression' && node.operator === 'typeof' ) {
const flattened = flatten( node.argument );
Expand All @@ -105,6 +151,38 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
}
}

// rewrite `require` (if not already handled) `global` and `define`, and handle free references to
// `module` and `exports` as these mean we need to wrap the module in commonjsHelpers.createCommonjsModule
if ( node.type === 'Identifier' ) {
if ( isReference( node, parent ) && !scope.contains( node.name ) ) {
if ( node.name in uses ) {
if ( node.name === 'require' ) {
if ( allowDynamicRequire ) return;
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, true );
}

uses[ node.name ] = true;
if ( node.name === 'global' && !ignoreGlobal ) {
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
}

// if module or exports are used outside the context of an assignment
// expression, we need to wrap the module
if ( node.name === 'module' || node.name === 'exports' ) {
shouldWrap = true;
}
}

if ( node.name === 'define' ) {
magicString.overwrite( node.start, node.end, 'undefined', true );
}

globals.add( node.name );
}

return;
}

// Is this an assignment to exports or module.exports?
if ( node.type === 'AssignmentExpression' ) {
if ( node.left.type !== 'MemberExpression' ) return;
Expand Down Expand Up @@ -137,68 +215,32 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
return;
}

if ( node.type === 'Identifier' ) {
if ( isReference( node, parent ) && !scope.contains( node.name ) ) {
if ( node.name in uses ) {
if ( node.name === 'require' ) {
if ( allowDynamicRequire ) return;
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsRequire`, true );
}

uses[ node.name ] = true;
if ( node.name === 'global' && !ignoreGlobal ) {
magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
}
// if this is `var x = require('x')`, we can do `import x from 'x'`
if ( node.type === 'VariableDeclarator' && node.id.type === 'Identifier' && isRequireStatement( node.init ) ) {
// for now, only do this for top-level requires. maybe fix this in future
if ( scope.parent ) return;

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

if ( node.name === 'define' ) {
magicString.overwrite( node.start, node.end, 'undefined', true );
}
const r = getRequired( node.init, node.id.name );
r.importsDefault = true;

globals.add( node.name );
if ( r.name === node.id.name ) {
node._shouldRemove = true;
}

return;
}

if ( node.type === 'ThisExpression' && lexicalDepth === 0 ) {
uses.global = true;
if ( !ignoreGlobal ) magicString.overwrite( node.start, node.end, `${HELPERS_NAME}.commonjsGlobal`, true );
return;
}

if ( node.type !== 'CallExpression' ) return;
if ( node.callee.name !== 'require' || scope.contains( 'require' ) ) return;
if ( node.arguments.length !== 1 || node.arguments[0].type !== 'Literal' ) return; // TODO handle these weird cases?
if ( ignoreRequire( node.arguments[0].value ) ) return;

const source = node.arguments[0].value;
if ( !isRequireStatement( node ) ) return;

const existing = required[ source ];
if ( existing === undefined ) {
sources.push( source );
}
let name;
const r = getRequired( node );

if ( !existing ) {
name = `require$$${uid++}`;
required[ source ] = { source, name, importsDefault: false };
} else {
name = required[ source ].name;
}

if ( parent.type !== 'ExpressionStatement' ) {
required[ source ].importsDefault = true;
magicString.overwrite( node.start, node.end, name );
} else {
if ( parent.type === 'ExpressionStatement' ) {
// is a bare import, e.g. `require('foo');`
magicString.remove( parent.start, parent.end );
} else {
r.importsDefault = true;
magicString.overwrite( node.start, node.end, r.name );
}

node.callee._skip = true;
Expand All @@ -208,6 +250,25 @@ export default function transformCommonjs ( code, id, isEntry, ignoreGlobal, ign
programDepth -= 1;
if ( node.scope ) scope = scope.parent;
if ( /^Function/.test( node.type ) ) lexicalDepth -= 1;

if ( node.type === 'VariableDeclaration' ) {
let keepDeclaration = false;

for ( let i = 0; i < node.declarations.length; i += 1 ) {
const declarator = node.declarations[i];
const next = node.declarations[ i + 1 ];

if ( declarator._shouldRemove ) {
magicString.remove( declarator.start, next ? next.start : declarator.end );
} else {
keepDeclaration = true;
}
}

if ( !keepDeclaration ) {
magicString.remove( node.start, node.end );
}
}
}
});

Expand Down
4 changes: 2 additions & 2 deletions test/form/ignore-ids-function/output.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import 'bar';
import require$$0 from 'commonjs-proxy:bar';
import bar from 'commonjs-proxy:bar';

var foo = require( 'foo' );
var bar = require$$0;


var input = {

Expand Down
4 changes: 2 additions & 2 deletions test/form/ignore-ids/output.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import 'bar';
import require$$0 from 'commonjs-proxy:bar';
import bar from 'commonjs-proxy:bar';

var foo = require( 'foo' );
var bar = require$$0;


var input = {

Expand Down

0 comments on commit c8c2548

Please sign in to comment.