Browse Source

warn if exporting an IIFE that looks like a function declaration, and wrap in parens if necessary (#1011)

gh-786
Rich-Harris 8 years ago
parent
commit
aefbed648c
  1. 23
      src/ast/nodes/ExportDefaultDeclaration.js
  2. 10
      test/function/warn-on-ambiguous-function-export/_config.js
  3. 6
      test/function/warn-on-ambiguous-function-export/foo.js
  4. 2
      test/function/warn-on-ambiguous-function-export/main.js
  5. 10
      test/function/wraps-ambiguous-default-export/_config.js
  6. 6
      test/function/wraps-ambiguous-default-export/foo.js
  7. 2
      test/function/wraps-ambiguous-default-export/main.js

23
src/ast/nodes/ExportDefaultDeclaration.js

@ -1,4 +1,6 @@
import Node from '../Node.js';
import getLocation from '../../utils/getLocation.js';
import relativeId from '../../utils/relativeId.js';
const functionOrClassDeclaration = /^(?:Function|Class)Declaration/;
@ -57,6 +59,27 @@ export default class ExportDefaultDeclaration extends Node {
}
if ( this.shouldInclude || this.declaration.activated ) {
if ( this.declaration.type === 'CallExpression' && this.declaration.callee.type === 'FunctionExpression' && this.declaration.arguments.length ) {
// we're exporting an IIFE. Check it doesn't look unintentional (#1011)
const isWrapped = /\(/.test( code.original.slice( this.start, this.declaration.start ) );
if ( !isWrapped ) {
code.insertRight( this.declaration.callee.start, '(' );
code.insertLeft( this.declaration.callee.end, ')' );
const start = this.declaration.callee.end;
let end = this.declaration.arguments[0].start - 1;
while ( code.original[ end ] !== '(' ) end -= 1;
const newlineSeparated = /\n/.test( code.original.slice( start, end ) );
if ( newlineSeparated ) {
const { line, column } = getLocation( this.module.code, this.declaration.start );
this.module.bundle.onwarn( `${relativeId( this.module.id )} (${line}:${column}) Ambiguous default export (is a call expression, but looks like a function declaration). See https://github.com/rollup/rollup/wiki/Troubleshooting#ambiguous-default-export` );
}
}
}
if ( this.activated ) {
if ( functionOrClassDeclaration.test( this.declaration.type ) ) {
if ( this.declaration.id ) {

10
test/function/warn-on-ambiguous-function-export/_config.js

@ -0,0 +1,10 @@
const assert = require( 'assert' );
module.exports = {
description: 'uses original name of default export function (#1011)',
warnings: warnings => {
assert.deepEqual( warnings, [
'foo.js (1:15) Ambiguous default export (is a call expression, but looks like a function declaration). See https://github.com/rollup/rollup/wiki/Troubleshooting#ambiguous-default-export'
]);
}
};

6
test/function/warn-on-ambiguous-function-export/foo.js

@ -0,0 +1,6 @@
export default function foo ( a, b ) {
assert.equal( a, b );
return 3;
}
( 1 + 1, 2 );

2
test/function/warn-on-ambiguous-function-export/main.js

@ -0,0 +1,2 @@
import x from './foo.js';
assert.equal( x, 3 );

10
test/function/wraps-ambiguous-default-export/_config.js

@ -0,0 +1,10 @@
const assert = require( 'assert' );
module.exports = {
description: 'wraps a function expression callee in parens to avoid it being parsed as function declaration (#1011)',
warnings: warnings => {
assert.deepEqual( warnings, [
'foo.js (1:15) Ambiguous default export (is a call expression, but looks like a function declaration). See https://github.com/rollup/rollup/wiki/Troubleshooting#ambiguous-default-export'
]);
}
};

6
test/function/wraps-ambiguous-default-export/foo.js

@ -0,0 +1,6 @@
export default function foo ( x ) {
assert.equal( x, 42 );
global.ran = true;
}
( 42 );

2
test/function/wraps-ambiguous-default-export/main.js

@ -0,0 +1,2 @@
import './foo.js';
assert.ok( global.ran );
Loading…
Cancel
Save