Skip to content

Commit

Permalink
Make sure errors from dynamic imports can be caught (#2958)
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Jun 22, 2019
1 parent dc7fc34 commit 9cb67d2
Show file tree
Hide file tree
Showing 41 changed files with 94 additions and 62 deletions.
18 changes: 14 additions & 4 deletions src/ast/nodes/Import.ts
Expand Up @@ -86,14 +86,24 @@ export default class Import extends NodeBase {
private getDynamicImportMechanism(options: RenderOptions): DynamicImportMechanism | null {
switch (options.format) {
case 'cjs': {
const _ = options.compact ? '' : ' ';
const resolve = options.compact ? 'c' : 'resolve';
switch (this.exportMode) {
case 'default':
const _ = options.compact ? '' : ' ';
return { left: `Promise.resolve({${_}'default':${_}require(`, right: `)${_}})` };
return {
left: `new Promise(function${_}(${resolve})${_}{${_}${resolve}({${_}'default':${_}require(`,
right: `)${_}});${_}})`
};
case 'auto':
return { left: `Promise.resolve(${INTEROP_NAMESPACE_VARIABLE}(require(`, right: ')))' };
return {
left: `new Promise(function${_}(${resolve})${_}{${_}${resolve}(${INTEROP_NAMESPACE_VARIABLE}(require(`,
right: `)));${_}})`
};
default:
return { left: 'Promise.resolve(require(', right: '))' };
return {
left: `new Promise(function${_}(${resolve})${_}{${_}${resolve}(require(`,
right: `));${_}})`
};
}
}
case 'amd': {
Expand Down
@@ -1,5 +1,5 @@
'use strict';

console.log('main1');
Promise.resolve(require('./generated-main4.dynamic.js'));
Promise.resolve(require('./generated-main5.js'));
new Promise(function (resolve) { resolve(require('./generated-main4.dynamic.js')); });
new Promise(function (resolve) { resolve(require('./generated-main5.js')); });
Expand Up @@ -5,4 +5,4 @@ var __chunk_1 = require('./nested/chunk.js');
var logo = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/logo1-25253976.svg').href : new URL('assets/logo1-25253976.svg', document.currentScript && document.currentScript.src || document.baseURI).href);

__chunk_1.showImage(logo);
Promise.resolve(require('./nested/chunk2.js'));
new Promise(function (resolve) { resolve(require('./nested/chunk2.js')); });
Expand Up @@ -6,4 +6,4 @@ const chunk = 'resolved';
const asset$1 = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-unresolved-9548436d.txt').href : new URL('assets/asset-unresolved-9548436d.txt', document.currentScript && document.currentScript.src || document.baseURI).href);
const chunk$1 = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/nested/chunk.js').href : new URL('nested/chunk.js', document.currentScript && document.currentScript.src || document.baseURI).href);

Promise.resolve(require('./nested/chunk2.js')).then(result => console.log(result, chunk, chunk$1, asset, asset$1));
new Promise(function (resolve) { resolve(require('./nested/chunk2.js')); }).then(result => console.log(result, chunk, chunk$1, asset, asset$1));
Expand Up @@ -4,4 +4,4 @@ var asset2 = 'resolved';

var asset3 = (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __dirname + '/assets/asset-unresolved-9548436d.txt').href : new URL('assets/asset-unresolved-9548436d.txt', document.currentScript && document.currentScript.src || document.baseURI).href);

Promise.resolve(require('./nested/chunk.js')).then(result => console.log(result, asset2, asset3));
new Promise(function (resolve) { resolve(require('./nested/chunk.js')); }).then(result => console.log(result, asset2, asset3));
@@ -1,4 +1,4 @@
'use strict';

console.log('dep1');
Promise.resolve(require('./generated-dep2.js'));
new Promise(function (resolve) { resolve(require('./generated-dep2.js')); });
@@ -1,4 +1,4 @@
'use strict';

console.log('main');
Promise.resolve(require('./generated-dep1.js'));
new Promise(function (resolve) { resolve(require('./generated-dep1.js')); });
Expand Up @@ -11,7 +11,7 @@ function fn (num) {
}

function dynamic (num) {
return Promise.resolve(require('./generated-dep2.js'))
return new Promise(function (resolve) { resolve(require('./generated-dep2.js')); })
.then(dep2 => {
return dep2.mult(num);
});
Expand Down
Expand Up @@ -19,6 +19,6 @@ function _interopNamespace(e) {
}
}

Promise.resolve(_interopNamespace(require(
new Promise(function (resolve) { resolve(_interopNamespace(require(
/* webpackChunkName: "chunk-name" */
'./foo.js'/*suffix*/)));
'./foo.js'/*suffix*/))); });
Expand Up @@ -21,4 +21,4 @@ function _interopNamespace(e) {

var dep = 'dep';

Promise.resolve(_interopNamespace(require(dep)));
new Promise(function (resolve) { resolve(_interopNamespace(require(dep))); });
Expand Up @@ -19,4 +19,4 @@ function _interopNamespace(e) {
}
}

Promise.resolve(_interopNamespace(require('./foo.js')));
new Promise(function (resolve) { resolve(_interopNamespace(require('./foo.js'))); });
@@ -1,3 +1,3 @@
'use strict';

Promise.resolve(require('./generated-dynamic.js')).then(({dynamic}) => console.log('main1', dynamic));
new Promise(function (resolve) { resolve(require('./generated-dynamic.js')); }).then(({dynamic}) => console.log('main1', dynamic));
Expand Up @@ -5,8 +5,8 @@ Object.defineProperty(exports, '__esModule', { value: true });
require('./generated-inlined.js');
require('./generated-separate.js');

const inlined = Promise.resolve(require('./generated-inlined.js'));
const separate = Promise.resolve(require('./generated-separate.js'));
const inlined = new Promise(function (resolve) { resolve(require('./generated-inlined.js')); });
const separate = new Promise(function (resolve) { resolve(require('./generated-separate.js')); });

exports.inlined = inlined;
exports.separate = separate;
Expand Up @@ -2,6 +2,6 @@

Object.defineProperty(exports, '__esModule', { value: true });

const separate = Promise.resolve(require('./generated-separate.js'));
const separate = new Promise(function (resolve) { resolve(require('./generated-separate.js')); });

exports.separate = separate;
Expand Up @@ -19,4 +19,4 @@ function _interopNamespace(e) {
}
}

Promise.resolve(_interopNamespace(require('./foo.js'))).then(result => console.log(result));
new Promise(function (resolve) { resolve(_interopNamespace(require('./foo.js'))); }).then(result => console.log(result));
@@ -1,5 +1,5 @@
'use strict';

var main = Promise.all([Promise.resolve({ 'default': require('./entry.js') }), Promise.resolve(require('./generated-other.js'))]);
var main = Promise.all([new Promise(function (resolve) { resolve({ 'default': require('./entry.js') }); }), new Promise(function (resolve) { resolve(require('./generated-other.js')); })]);

module.exports = main;
@@ -1,3 +1,3 @@
'use strict';

Promise.resolve(require('./generated-dynamic.js')).then(({ value }) => console.log(value));
new Promise(function (resolve) { resolve(require('./generated-dynamic.js')); }).then(({ value }) => console.log(value));
@@ -1,3 +1,3 @@
'use strict';

Promise.resolve(require('./generated-dep1.js')).then(({ bar }) => console.log(bar()));
new Promise(function (resolve) { resolve(require('./generated-dep1.js')); }).then(({ bar }) => console.log(bar()));
Expand Up @@ -6,7 +6,7 @@ var __chunk_1 = require('./chunks/chunk.js');

assert.equal(__chunk_1.sharedValue, 'shared');

const promise = Promise.resolve(require('./chunks/other.js')).then(result =>
const promise = new Promise(function (resolve) { resolve(require('./chunks/other.js')); }).then(result =>
assert.deepEqual(result, { value: 'shared' })
);

Expand Down
Expand Up @@ -10,6 +10,6 @@ const getWorkerMessage = () => new Promise(resolve => {
document.body.innerHTML += `<h1>main: ${__chunk_1.shared}</h1>`;
getWorkerMessage().then(message => (document.body.innerHTML += `<h1>1: ${message.data}</h1>`));

Promise.resolve(require('./chunks/nested.js'))
new Promise(function (resolve) { resolve(require('./chunks/nested.js')); })
.then(result => result.getWorkerMessage())
.then(message => (document.body.innerHTML += `<h1>2: ${message.data}</h1>`));
Expand Up @@ -3,4 +3,4 @@
var __chunk_1 = require('./nested/chunk.js');

__chunk_1.log('main: ' + (typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('main.js', document.baseURI).href)));
Promise.resolve(require('./nested/chunk2.js'));
new Promise(function (resolve) { resolve(require('./nested/chunk2.js')); });
Expand Up @@ -2,6 +2,6 @@

var dynamic = require('./generated-dynamic.js');

Promise.all([Promise.resolve(require('./generated-dynamic.js')), Promise.resolve(require('./generated-dynamic2.js')), Promise.resolve(require('./generated-dynamic3.js'))]).then(
Promise.all([new Promise(function (resolve) { resolve(require('./generated-dynamic.js')); }), new Promise(function (resolve) { resolve(require('./generated-dynamic2.js')); }), new Promise(function (resolve) { resolve(require('./generated-dynamic3.js')); })]).then(
results => console.log(results, dynamic.DEP)
);
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic1.js')).then(result => console.log(result));
Promise.resolve(require('./generated-dynamic.js')).then(result => console.log(result));
new Promise(function (resolve) { resolve(require('./generated-dynamic1.js')); }).then(result => console.log(result));
new Promise(function (resolve) { resolve(require('./generated-dynamic.js')); }).then(result => console.log(result));
@@ -1,3 +1,3 @@
'use strict';

Promise.resolve(require('./generated-dynamic.js')).then(({DYNAMIC_USED_BY_A}) => console.log(DYNAMIC_USED_BY_A));
new Promise(function (resolve) { resolve(require('./generated-dynamic.js')); }).then(({DYNAMIC_USED_BY_A}) => console.log(DYNAMIC_USED_BY_A));
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic2.js'));
new Promise(function (resolve) { resolve(require('./generated-dynamic2.js')); });
console.log('dynamic1');
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic3.js'));
new Promise(function (resolve) { resolve(require('./generated-dynamic3.js')); });
console.log('dynamic2');
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic4.js'));
new Promise(function (resolve) { resolve(require('./generated-dynamic4.js')); });
console.log('dynamic3');
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic5.js'));
new Promise(function (resolve) { resolve(require('./generated-dynamic5.js')); });
console.log('dynamic4');
@@ -1,4 +1,4 @@
'use strict';

Promise.resolve(require('./generated-dynamic1.js'));
new Promise(function (resolve) { resolve(require('./generated-dynamic1.js')); });
console.log('main');
@@ -1,3 +1,3 @@
'use strict';

Promise.resolve(require('./dynamic-included.js')).then(result => console.log(result));
new Promise(function (resolve) { resolve(require('./dynamic-included.js')); }).then(result => console.log(result));
Expand Up @@ -26,20 +26,20 @@ require('direct-absolute-external');
require('to-indirect-absolute-external');

// nested
Promise.resolve(require('./generated-existing.js'));
Promise.resolve(_interopNamespace(require('./direct-relative-external')));
Promise.resolve(_interopNamespace(require('to-indirect-relative-external')));
Promise.resolve(_interopNamespace(require('direct-absolute-external')));
Promise.resolve(_interopNamespace(require('to-indirect-absolute-external')));
new Promise(function (resolve) { resolve(require('./generated-existing.js')); });
new Promise(function (resolve) { resolve(_interopNamespace(require('./direct-relative-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('to-indirect-relative-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('direct-absolute-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('to-indirect-absolute-external'))); });

//main
Promise.resolve(require('./generated-existing.js'));
Promise.resolve(_interopNamespace(require('./direct-relative-external')));
Promise.resolve(_interopNamespace(require('to-indirect-relative-external')));
Promise.resolve(_interopNamespace(require('direct-absolute-external')));
Promise.resolve(_interopNamespace(require('to-indirect-absolute-external')));
new Promise(function (resolve) { resolve(require('./generated-existing.js')); });
new Promise(function (resolve) { resolve(_interopNamespace(require('./direct-relative-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('to-indirect-relative-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('direct-absolute-external'))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('to-indirect-absolute-external'))); });

Promise.resolve(_interopNamespace(require('dynamic-direct-external' + unknown)));
Promise.resolve(_interopNamespace(require('to-dynamic-indirect-external')));
Promise.resolve(require('./generated-existing.js'));
Promise.resolve(_interopNamespace(require('my' + 'replacement')));
new Promise(function (resolve) { resolve(_interopNamespace(require('dynamic-direct-external' + unknown))); });
new Promise(function (resolve) { resolve(_interopNamespace(require('to-dynamic-indirect-external'))); });
new Promise(function (resolve) { resolve(require('./generated-existing.js')); });
new Promise(function (resolve) { resolve(_interopNamespace(require('my' + 'replacement'))); });
Expand Up @@ -4,7 +4,7 @@ Object.defineProperty(exports, '__esModule', { value: true });

var __chunk_1 = require('./generated-chunk.js');

const lazy = Promise.resolve(require('./generated-lazy.js'));
const lazy = new Promise(function (resolve) { resolve(require('./generated-lazy.js')); });

exports.v1 = __chunk_1.v1;
exports.v10 = __chunk_1.v10;
Expand Down
Expand Up @@ -34,7 +34,7 @@ const document$1 = 1;
const URL$1 = 1;
console.log(_interopDefault$1, _interopNamespace$1, module$1, require$1, exports$1, document$1, URL$1);

Promise.resolve(_interopNamespace(require('external')));
new Promise(function (resolve) { resolve(_interopNamespace(require('external'))); });
exports.default = 0;
console.log((typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('cjs.js', document.baseURI).href)));

Expand All @@ -48,7 +48,7 @@ function nested1() {
const URL$1 = 1;
console.log(_interopDefault, _interopNamespace$1, module, require$1, exports$1, document$1, URL$1);

Promise.resolve(_interopNamespace(require('external')));
new Promise(function (resolve) { resolve(_interopNamespace(require('external'))); });
exports.default = 1;
console.log((typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('cjs.js', document.baseURI).href)));
}
Expand Down
Expand Up @@ -27,7 +27,7 @@ var myExternal = _interopDefault(require('external'));

const test = () => myExternal;

const someDynamicImport = () => Promise.resolve(_interopNamespace(require('external')));
const someDynamicImport = () => new Promise(function (resolve) { resolve(_interopNamespace(require('external'))); });

exports.someDynamicImport = someDynamicImport;
exports.test = test;
16 changes: 16 additions & 0 deletions test/function/samples/catch-dynamic-import-failure/_config.js
@@ -0,0 +1,16 @@
const assert = require('assert');

module.exports = {
description: 'allows catching failed dynamic imports',
options: {
input: ['main', 'exists-default'],
external: ['does-not-exist']
},
exports(exports) {
return exports.then(result => {
assert.strictEqual(result[0].message, 'exists-named');
assert.strictEqual(result[1].message, 'exists-default');
assert.strictEqual(result[2].message, "Cannot find module 'does-not-exist'");
});
}
};
@@ -0,0 +1,2 @@
export default 42;
throw new Error('exists-default');
@@ -0,0 +1,2 @@
export const value = 42;
throw new Error('exists-named');
5 changes: 5 additions & 0 deletions test/function/samples/catch-dynamic-import-failure/main.js
@@ -0,0 +1,5 @@
export default Promise.all([
import('./exists-named.js').catch(err => err),
import('./exists-default.js').catch(err => err),
import('does-not-exist').catch(err => err)
]);
4 changes: 2 additions & 2 deletions test/function/samples/dynamic-import-expression/_config.js
Expand Up @@ -23,7 +23,7 @@ module.exports = {
}
]
},
runtimeError(error) {
assert.equal(error.message.split('\n')[0], "Cannot find module 'x/y'");
exports(exports) {
return exports.catch(err => assert.strictEqual(err.message, "Cannot find module 'x/y'"));
}
};
2 changes: 1 addition & 1 deletion test/function/samples/dynamic-import-expression/main.js
@@ -1 +1 @@
import( 'x/' + 'y' )
export default import('x/' + 'y');
5 changes: 1 addition & 4 deletions test/function/samples/dynamic-import-rewriting/_config.js
Expand Up @@ -13,9 +13,6 @@ module.exports = {
]
},
exports(exports) {
return exports.promise;
},
runtimeError(error) {
assert.equal(error.message.split('\n')[0], "Cannot find module 'asdf'");
return exports.promise.catch(err => assert.equal(err.message, "Cannot find module 'asdf'"));
}
};

0 comments on commit 9cb67d2

Please sign in to comment.