Handle namespaces that reexport themselves (#3077)

* Create test

* Separate namespace initialisation from creation to prevent an infinite recursion

* Throw for unexpected warnings in form tests

This should also reduce test noise a lot

* Update dependencies and fix vulnerability

* Add test that guesses variable names to improve coverage again

* Test that warnings can be cast to string

* Retrigger CI

package.json

@@ -66,17 +66,17 @@
     "@types/micromatch": "^3.1.0",
     "@types/minimist": "^1.2.0",
     "acorn-import-meta": "^1.0.0",
-    "acorn-jsx": "^5.0.1",
+    "acorn-jsx": "^5.0.2",
     "acorn-walk": "^7.0.0",
     "buble": "^0.19.8",
-    "chokidar": "^2.1.6",
+    "chokidar": "^2.1.8",
     "codecov": "^3.5.0",
     "console-group": "^0.3.3",
     "core-js": "^3.2.1",
     "date-time": "^3.1.0",
     "es5-shim": "^4.5.13",
     "es6-shim": "^0.35.5",
-    "eslint": "^6.1.0",
+    "eslint": "^6.2.1",
     "eslint-plugin-import": "^2.18.2",
     "execa": "^2.0.4",
     "fixturify": "^1.2.0",
@@ -87,7 +87,7 @@
     "lint-staged": "^9.2.3",
     "locate-character": "^2.0.5",
     "magic-string": "^0.25.3",
-    "markdownlint-cli": "^0.17.0",
+    "markdownlint-cli": "^0.18.0",
     "micromatch": "^4.0.2",
     "minimist": "^1.2.0",
     "mocha": "^6.2.0",
@@ -97,8 +97,8 @@
     "pretty-ms": "^5.0.0",
     "require-relative": "^0.8.7",
     "requirejs": "^2.3.6",
-    "rollup": "^1.19.4",
-    "rollup-plugin-alias": "^1.5.2",
+    "rollup": "^1.20.1",
+    "rollup-plugin-alias": "^2.0.0",
     "rollup-plugin-buble": "^0.19.8",
     "rollup-plugin-commonjs": "^10.0.2",
     "rollup-plugin-json": "^4.0.0",
@@ -116,9 +116,9 @@
     "source-map-support": "^0.5.13",
     "sourcemap-codec": "^1.4.6",
     "systemjs": "^5.0.0",
-    "terser": "^4.1.4",
+    "terser": "^4.2.0",
     "tslib": "^1.10.0",
-    "tslint": "^5.18.0",
+    "tslint": "^5.19.0",
     "turbocolor": "^2.6.1",
     "typescript": "^3.5.3",
     "url-parse": "^1.4.7"

rollup.config.js

@@ -135,8 +135,10 @@ function fixAcornEsmImport() {
 
 const moduleAliases = {
 	resolve: ['.js', '.json', '.md'],
-	'help.md': path.resolve('cli/help.md'),
-	'package.json': path.resolve('package.json')
+	entries: [
+		{ find: 'help.md', replacement: path.resolve('cli/help.md') },
+		{ find: 'package.json', replacement: path.resolve('package.json') }
+	]
 };
 
 const treeshake = {

src/Module.ts

@@ -342,9 +342,11 @@ export default class Module {
 	}
 
 	getOrCreateNamespace(): NamespaceVariable {
-		return (
-			this.namespaceVariable || (this.namespaceVariable = new NamespaceVariable(this.astContext))
-		);
+		if (!this.namespaceVariable) {
+			this.namespaceVariable = new NamespaceVariable(this.astContext);
+			this.namespaceVariable.initialise();
+		}
+		return this.namespaceVariable;
 	}
 
 	getReexports(): string[] {

src/ast/variables/NamespaceVariable.ts

@@ -19,10 +19,6 @@ export default class NamespaceVariable extends Variable {
 		super(context.getModuleName());
 		this.context = context;
 		this.module = context.module;
-		for (const name of this.context.getExports().concat(this.context.getReexports())) {
-			if (name[0] === '*' && name.length > 1) this.containsExternalNamespace = true;
-			this.memberVariables[name] = this.context.traceExport(name);
-		}
 	}
 
 	addReference(identifier: Identifier) {
@@ -69,6 +65,13 @@ export default class NamespaceVariable extends Variable {
 		}
 	}
 
+	initialise() {
+		for (const name of this.context.getExports().concat(this.context.getReexports())) {
+			if (name[0] === '*' && name.length > 1) this.containsExternalNamespace = true;
+			this.memberVariables[name] = this.context.traceExport(name);
+		}
+	}
+
 	renderBlock(options: RenderOptions) {
 		const _ = options.compact ? '' : ' ';
 		const n = options.compact ? '' : '\n';

test/form/index.js

@@ -21,10 +21,15 @@ runTestSuiteWithSamples('form', path.resolve(__dirname, 'samples'), (dir, config
 						extend(
 							{
 								input: dir + '/main.js',
-								onwarn: msg => {
-									if (/No name was provided for/.test(msg)) return;
-									if (/as external dependency/.test(msg)) return;
-									console.error(msg);
+								onwarn: warning => {
+									if (
+										!(config.expectedWarnings && config.expectedWarnings.indexOf(warning.code) >= 0)
+									) {
+										throw new Error(
+											`Unexpected warnings (${warning.code}): ${warning.message}\n` +
+												'If you expect warnings, list their codes in config.expectedWarnings'
+										);
+									}
 								},
 								strictDeprecations: true
 							},

test/form/samples/builtin-prototypes/object-expression/_config.js

@@ -1,3 +1,4 @@
 module.exports = {
-	description: 'Tree-shake known object prototype functions'
+	description: 'Tree-shake known object prototype functions',
+	expectedWarnings: ['EMPTY_BUNDLE']
 };

test/form/samples/compact-multiple-imports/_config.js

@@ -5,6 +5,10 @@ module.exports = {
 			return id.startsWith('external');
 		},
 		output: {
+			globals: {
+				'external-3': 'external3',
+				'external-4': 'external4'
+			},
 			compact: true
 		}
 	}

test/form/samples/compact/_config.js

@@ -1,11 +1,15 @@
 module.exports = {
 	description: 'compact output with compact: true',
+	expectedWarnings: ['CIRCULAR_DEPENDENCY'],
 	options: {
 		external: ['external'],
 		output: {
 			name: 'foo',
 			compact: true,
-			namespaceToStringTag: true
+			namespaceToStringTag: true,
+			globals: {
+				external: 'x'
+			}
 		}
 	}
 };

test/form/samples/conflicting-imports/_config.js

@@ -1,6 +1,12 @@
 module.exports = {
 	description: 'ensures bundle imports are deconflicted (#659)',
 	options: {
-		external: ['foo', 'bar']
+		external: ['foo', 'bar'],
+		output: {
+			globals: {
+				bar: 'bar',
+				foo: 'foo'
+			}
+		}
 	}
 };

test/form/samples/custom-module-context-function/_config.js

@@ -1,5 +1,6 @@
 module.exports = {
 	description: 'allows custom module-specific context with a function option',
+	expectedWarnings: ['THIS_IS_UNDEFINED'],
 	options: {
 		moduleContext(id) {
 			return /main\.js$/.test(id) ? 'lolwut' : 'undefined';

test/form/samples/custom-module-context/_config.js

@@ -1,5 +1,6 @@
 module.exports = {
 	description: 'allows custom module-specific context',
+	expectedWarnings: ['THIS_IS_UNDEFINED'],
 	options: {
 		moduleContext: {
 			'main.js': 'lolwut'

test/form/samples/deconflict-format-specific-globals/_config.js

@@ -3,6 +3,7 @@ module.exports = {
 	options: {
 		external: 'external',
 		output: {
+			globals: { external: 'external' },
 			name: 'bundle'
 		}
 	}

test/form/samples/dedupes-external-imports/_config.js

@@ -3,6 +3,7 @@ module.exports = {
 	options: {
 		external: ['external'],
 		output: {
+			globals: { external: 'external' },
 			name: 'myBundle'
 		}
 	}

test/form/samples/deprecated/prune-pure-unused-import-array/_config.js

@@ -1,8 +1,11 @@
 module.exports = {
+	description: 'prunes pure unused external imports ([#1352])',
 	options: {
 		strictDeprecations: false,
 		external: ['external', 'other'],
-		treeshake: { pureExternalModules: ['external'] }
-	},
-	description: 'prunes pure unused external imports ([#1352])'
+		treeshake: { pureExternalModules: ['external'] },
+		output: {
+			globals: { other: 'other' }
+		}
+	}
 };

test/form/samples/deprecated/prune-pure-unused-import-function/_config.js

@@ -1,8 +1,12 @@
 module.exports = {
+	description: 'prunes pure unused external imports ([#1352])',
+	expectedWarnings: ['EMPTY_BUNDLE'],
 	options: {
 		strictDeprecations: false,
 		external: ['external', 'other'],
-		treeshake: { pureExternalModules: id => id === 'external' }
-	},
-	description: 'prunes pure unused external imports ([#1352])'
+		treeshake: { pureExternalModules: id => id === 'external' },
+		output: {
+			globals: { other: 'other' }
+		}
+	}
 };

test/form/samples/deprecated/prune-pure-unused-import/_config.js

@@ -1,8 +1,9 @@
 module.exports = {
+	description: 'prunes pure unused external imports ([#1352])',
+	expectedWarnings: ['EMPTY_BUNDLE'],
 	options: {
 		strictDeprecations: false,
 		external: ['external', 'other'],
 		treeshake: { pureExternalModules: true }
-	},
-	description: 'prunes pure unused external imports ([#1352])'
+	}
 };

test/form/samples/deprecated/transform-bundle-plugin-options/_config.js

@@ -2,6 +2,7 @@ const assert = require('assert');
 
 module.exports = {
 	description: 'plugin .transformBundle gets passed options',
+	expectedWarnings: ['DEPRECATED_FEATURE'],
 	options: {
 		strictDeprecations: false,
 		plugins: [

test/form/samples/deprecated/transform-bundle-plugin/_config.js

@@ -1,10 +1,11 @@
 module.exports = {
 	description: 'allows plugins to transform bundle',
+	expectedWarnings: ['DEPRECATED_FEATURE'],
 	options: {
 		strictDeprecations: false,
 		plugins: [
 			{
-				transformBundle(code) {
+				transformBundle() {
 					return '/* first plugin */';
 				}
 			},

test/form/samples/export-all-before-named/_config.js

@@ -1,7 +1,10 @@
 module.exports = {
 	description: 'external `export *` must not interfere with internal exports',
 	options: {
-		output: { name: 'exposedInternals' },
+		output: {
+			globals: { external: 'external' },
+			name: 'exposedInternals'
+		},
 		external: ['external']
 	}
 };

test/form/samples/export-default-import/_config.js

@@ -2,6 +2,9 @@ module.exports = {
 	description: 'correctly exports a default import, even in ES mode (#513)',
 	options: {
 		external: ['x'],
-		output: { name: 'myBundle' }
+		output: {
+			globals: { x: 'x' },
+			name: 'myBundle'
+		}
 	}
 };

test/form/samples/external-deshadowing/_config.js

@@ -3,6 +3,7 @@ module.exports = {
 	options: {
 		external: ['a', 'b'],
 		output: {
+			globals: { a: 'a', b: 'b' },
 			name: 'myBundle'
 		}
 	}

test/form/samples/external-deshadowing/_expected/iife.js

@@ -16,4 +16,4 @@ var myBundle = (function (exports, a, Test$1) {
 
   return exports;
 
-}({}, a, Test$1));
+}({}, a, b));

test/form/samples/external-deshadowing/_expected/umd.js

@@ -1,7 +1,7 @@
 (function (global, factory) {
   typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('a'), require('b')) :
   typeof define === 'function' && define.amd ? define(['exports', 'a', 'b'], factory) :
-  (global = global || self, factory(global.myBundle = {}, global.a, global.Test$1));
+  (global = global || self, factory(global.myBundle = {}, global.a, global.b));
 }(this, function (exports, a, Test$1) { 'use strict';
 
   Test$1 = Test$1 && Test$1.hasOwnProperty('default') ? Test$1['default'] : Test$1;

test/form/samples/external-export-tracing/_config.js

@@ -3,6 +3,7 @@ module.exports = {
 	options: {
 		external: ['external'],
 		output: {
+			globals: { external: 'external' },
 			name: 'myBundle'
 		}
 	}

test/form/samples/external-import-alias-shadow/_config.js

@@ -1,6 +1,9 @@
 module.exports = {
 	description: 'handles external aliased named imports that shadow another name',
 	options: {
-		external: ['acorn']
+		external: ['acorn'],
+		output: {
+			globals: { acorn: 'acorn' }
+		}
 	}
 };

test/form/samples/external-imports/_config.js

@@ -1,6 +1,15 @@
 module.exports = {
 	description: 'prefixes global names with `global.` when creating UMD bundle (#57)',
+	expectedWarnings: ['UNUSED_EXTERNAL_IMPORT'],
 	options: {
-		external: ['factory', 'baz', 'shipping-port', 'alphabet']
+		external: ['factory', 'baz', 'shipping-port', 'alphabet'],
+		output: {
+			globals: {
+				alphabet: 'alphabet',
+				baz: 'baz',
+				factory: 'factory',
+				'shipping-port': 'containers'
+			}
+		}
 	}
 };

test/form/samples/external-namespace-and-named/_config.js

@@ -1,6 +1,9 @@
 module.exports = {
 	description: 'Correctly handles external namespace tracing with both namespace and named exports',
 	options: {
-		external: ['foo']
+		external: ['foo'],
+		output: {
+			globals: { foo: 'foo' }
+		}
 	}
 };

test/form/samples/external-namespace-reexport/_config.js

@@ -3,6 +3,7 @@ module.exports = {
 	options: {
 		external: ['highcharts'],
 		output: {
+			globals: { highcharts: 'highcharts' },
 			name: 'myBundle'
 		}
 	}

test/form/samples/guessed-global-names/_config.js

@@ -0,0 +1,15 @@
+const { resolve } = require('path');
+
+module.exports = {
+	description: 'guesses global names if necessary',
+	expectedWarnings: ['MISSING_GLOBAL_NAME'],
+	options: {
+		external: [
+			'unchanged',
+			'changed',
+			'special-character',
+			'with/slash',
+			resolve(__dirname, 'relative.js')
+		]
+	}
+};

test/form/samples/guessed-global-names/_expected/amd.js

@@ -0,0 +1,7 @@
+define(['unchanged', 'changed', 'special-character', 'with/slash', './relative'], function (unchanged, changedName, specialCharacter, slash, relative_js) { 'use strict';
+
+	changedName = changedName && changedName.hasOwnProperty('default') ? changedName['default'] : changedName;
+
+	console.log(unchanged.foo, changedName, specialCharacter.bar, slash.baz, relative_js.quux);
+
+});