From 028e425b91d8f27f6acdb34fc4599a1ae798b938 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Wed, 28 Dec 2016 13:38:28 -0500 Subject: [PATCH 1/2] more informative warning for implicit external dependencies --- bin/src/runRollup.js | 3 ++- src/Bundle.js | 3 ++- test/function/custom-path-resolver-async/_config.js | 2 +- test/function/custom-path-resolver-sync/_config.js | 2 +- test/function/does-not-hang-on-missing-module/_config.js | 8 ++++---- test/function/unused-import/_config.js | 2 +- 6 files changed, 11 insertions(+), 9 deletions(-) diff --git a/bin/src/runRollup.js b/bin/src/runRollup.js index 84cd3da..63bbd0c 100644 --- a/bin/src/runRollup.js +++ b/bin/src/runRollup.js @@ -60,7 +60,8 @@ export default function runRollup ( command ) { rollup.rollup({ entry: config, onwarn: message => { - if ( /Treating .+ as external dependency/.test( message ) ) return; + // TODO use warning codes instead of this hackery + if ( /treating it as an external dependency/.test( message ) ) return; stderr( message ); } }).then( bundle => { diff --git a/src/Bundle.js b/src/Bundle.js index 3a1774b..1166cb2 100644 --- a/src/Bundle.js +++ b/src/Bundle.js @@ -16,6 +16,7 @@ import transform from './utils/transform.js'; import transformBundle from './utils/transformBundle.js'; import collapseSourcemaps from './utils/collapseSourcemaps.js'; import callIfFunction from './utils/callIfFunction.js'; +import relativeId from './utils/relativeId.js'; import { dirname, isRelative, isAbsolute, normalize, relative, resolve } from './utils/path.js'; import BundleScope from './ast/scopes/BundleScope.js'; @@ -332,7 +333,7 @@ export default class Bundle { if ( !resolvedId && !isExternal ) { if ( isRelative( source ) ) throw new Error( `Could not resolve '${source}' from ${module.id}` ); - this.onwarn( `Treating '${source}' as external dependency` ); + this.onwarn( `'${source}' is imported by ${relativeId( module.id )}, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` ); isExternal = true; } diff --git a/test/function/custom-path-resolver-async/_config.js b/test/function/custom-path-resolver-async/_config.js index a0df42f..83a10c1 100644 --- a/test/function/custom-path-resolver-async/_config.js +++ b/test/function/custom-path-resolver-async/_config.js @@ -23,7 +23,7 @@ module.exports = { }, warnings: function ( warnings ) { assert.deepEqual( warnings, [ - "Treating 'path' as external dependency" + `'path' is imported by main.js, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` ]); }, exports: function ( exports ) { diff --git a/test/function/custom-path-resolver-sync/_config.js b/test/function/custom-path-resolver-sync/_config.js index b676f5d..074f671 100644 --- a/test/function/custom-path-resolver-sync/_config.js +++ b/test/function/custom-path-resolver-sync/_config.js @@ -15,7 +15,7 @@ module.exports = { }, warnings: function ( warnings ) { assert.deepEqual( warnings, [ - "Treating 'path' as external dependency" + `'path' is imported by main.js, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` ]); }, exports: function ( exports ) { diff --git a/test/function/does-not-hang-on-missing-module/_config.js b/test/function/does-not-hang-on-missing-module/_config.js index f5b0cfa..901bfae 100644 --- a/test/function/does-not-hang-on-missing-module/_config.js +++ b/test/function/does-not-hang-on-missing-module/_config.js @@ -2,10 +2,10 @@ var assert = require( 'assert' ); module.exports = { description: 'does not hang on missing module (#53)', - options: { - onwarn: function ( msg ) { - assert.equal( "Treating 'unlessYouCreatedThisFileForSomeReason' as external dependency", msg ); - } + warnings: warnings => { + assert.deepEqual( warnings, [ + `'unlessYouCreatedThisFileForSomeReason' is imported by main.js, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency` + ]); }, runtimeError: function ( error ) { assert.equal( "Cannot find module 'unlessYouCreatedThisFileForSomeReason'", error.message ); diff --git a/test/function/unused-import/_config.js b/test/function/unused-import/_config.js index 6515768..b9e9b85 100644 --- a/test/function/unused-import/_config.js +++ b/test/function/unused-import/_config.js @@ -4,7 +4,7 @@ module.exports = { description: 'warns on unused imports ([#595])', warnings: warnings => { assert.deepEqual( warnings, [ - `Treating 'external' as external dependency`, + `'external' is imported by main.js, but could not be resolved – treating it as an external dependency. For help see https://github.com/rollup/rollup/wiki/Troubleshooting#treating-module-as-external-dependency`, `'unused', 'notused' and 'neverused' are imported from external module 'external' but never used`, `Generated an empty bundle` ]); From b1a9abc261d29d9eba6f991a019f80cc8d32b1be Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Wed, 28 Dec 2016 14:40:54 -0500 Subject: [PATCH 2/2] use path.relative inside relativeId, to try and fix tests on windows --- src/utils/relativeId.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/utils/relativeId.js b/src/utils/relativeId.js index e26bd02..f6ffdeb 100644 --- a/src/utils/relativeId.js +++ b/src/utils/relativeId.js @@ -1,4 +1,6 @@ +import { isAbsolute, relative } from './path.js'; + export default function relativeId ( id ) { - if ( typeof process === 'undefined' ) return id; - return id.replace( process.cwd(), '' ).replace( /^[\/\\]/, '' ); + if ( typeof process === 'undefined' || !isAbsolute( id ) ) return id; + return relative( process.cwd(), id ); }