From f4b19ab5c90c98af1108fc1aee27ebbeb432cf63 Mon Sep 17 00:00:00 2001 From: Alec Larson Date: Sun, 16 Jun 2019 00:59:30 -0400 Subject: [PATCH] Improve handling of conflicting namespace exports (#2893) * test: "export *" before named export * Prioritize local exports over external exports when resolving export all modules --- src/Module.ts | 17 +++++++++----- .../export-all-before-named/_config.js | 7 ++++++ .../export-all-before-named/_expected/amd.js | 19 +++++++++++++++ .../export-all-before-named/_expected/cjs.js | 19 +++++++++++++++ .../export-all-before-named/_expected/es.js | 7 ++++++ .../export-all-before-named/_expected/iife.js | 20 ++++++++++++++++ .../_expected/system.js | 22 ++++++++++++++++++ .../export-all-before-named/_expected/umd.js | 23 +++++++++++++++++++ .../export-all-before-named/internal.js | 3 +++ .../samples/export-all-before-named/main.js | 2 ++ 10 files changed, 133 insertions(+), 6 deletions(-) create mode 100644 test/form/samples/export-all-before-named/_config.js create mode 100644 test/form/samples/export-all-before-named/_expected/amd.js create mode 100644 test/form/samples/export-all-before-named/_expected/cjs.js create mode 100644 test/form/samples/export-all-before-named/_expected/es.js create mode 100644 test/form/samples/export-all-before-named/_expected/iife.js create mode 100644 test/form/samples/export-all-before-named/_expected/system.js create mode 100644 test/form/samples/export-all-before-named/_expected/umd.js create mode 100644 test/form/samples/export-all-before-named/internal.js create mode 100644 test/form/samples/export-all-before-named/main.js diff --git a/src/Module.ts b/src/Module.ts index b27baf2d7fe..eab9eecd9f8 100644 --- a/src/Module.ts +++ b/src/Module.ts @@ -429,8 +429,7 @@ export default class Module { } if (name !== 'default') { - for (let i = 0; i < this.exportAllModules.length; i += 1) { - const module = this.exportAllModules[i]; + for (const module of this.exportAllModules) { const declaration = module.getVariableForExportName(name, true); if (declaration) return declaration; @@ -504,10 +503,16 @@ export default class Module { this.addModulesToSpecifiers(this.importDescriptions); this.addModulesToSpecifiers(this.reexports); - this.exportAllModules = this.exportAllSources.map(source => { - const id = this.resolvedIds[source].id; - return this.graph.moduleById.get(id) as any; - }); + this.exportAllModules = this.exportAllSources + .map(source => { + const id = this.resolvedIds[source].id; + return this.graph.moduleById.get(id) as Module | ExternalModule; + }) + .sort((moduleA, moduleB) => { + const aExternal = moduleA instanceof ExternalModule; + const bExternal = moduleB instanceof ExternalModule; + return aExternal === bExternal ? 0 : aExternal ? 1 : -1; + }); } render(options: RenderOptions): MagicString { diff --git a/test/form/samples/export-all-before-named/_config.js b/test/form/samples/export-all-before-named/_config.js new file mode 100644 index 00000000000..b738dc58cd2 --- /dev/null +++ b/test/form/samples/export-all-before-named/_config.js @@ -0,0 +1,7 @@ +module.exports = { + description: 'external `export *` must not interfere with internal exports', + options: { + output: { name: 'exposedInternals' }, + external: ['external'] + } +}; diff --git a/test/form/samples/export-all-before-named/_expected/amd.js b/test/form/samples/export-all-before-named/_expected/amd.js new file mode 100644 index 00000000000..287823703f2 --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/amd.js @@ -0,0 +1,19 @@ +define(['exports', 'external'], function (exports, external) { 'use strict'; + + function internalFn(path) { + return path[0] === '.'; + } + + Object.keys(external).forEach(function (key) { + Object.defineProperty(exports, key, { + enumerable: true, + get: function () { + return external[key]; + } + }); + }); + exports.internalFn = internalFn; + + Object.defineProperty(exports, '__esModule', { value: true }); + +}); diff --git a/test/form/samples/export-all-before-named/_expected/cjs.js b/test/form/samples/export-all-before-named/_expected/cjs.js new file mode 100644 index 00000000000..876085e6b6c --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/cjs.js @@ -0,0 +1,19 @@ +'use strict'; + +Object.defineProperty(exports, '__esModule', { value: true }); + +var external = require('external'); + +function internalFn(path) { + return path[0] === '.'; +} + +Object.keys(external).forEach(function (key) { + Object.defineProperty(exports, key, { + enumerable: true, + get: function () { + return external[key]; + } + }); +}); +exports.internalFn = internalFn; diff --git a/test/form/samples/export-all-before-named/_expected/es.js b/test/form/samples/export-all-before-named/_expected/es.js new file mode 100644 index 00000000000..2339e751c6f --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/es.js @@ -0,0 +1,7 @@ +export * from 'external'; + +function internalFn(path) { + return path[0] === '.'; +} + +export { internalFn }; diff --git a/test/form/samples/export-all-before-named/_expected/iife.js b/test/form/samples/export-all-before-named/_expected/iife.js new file mode 100644 index 00000000000..55d0cb30fa9 --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/iife.js @@ -0,0 +1,20 @@ +var exposedInternals = (function (exports, external) { + 'use strict'; + + function internalFn(path) { + return path[0] === '.'; + } + + Object.keys(external).forEach(function (key) { + Object.defineProperty(exports, key, { + enumerable: true, + get: function () { + return external[key]; + } + }); + }); + exports.internalFn = internalFn; + + return exports; + +}({}, external)); diff --git a/test/form/samples/export-all-before-named/_expected/system.js b/test/form/samples/export-all-before-named/_expected/system.js new file mode 100644 index 00000000000..3325a7ce5ed --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/system.js @@ -0,0 +1,22 @@ +System.register('exposedInternals', ['external'], function (exports) { + 'use strict'; + var _starExcludes = { internalFn: 1, default: 1 }; + return { + setters: [function (module) { + var _setter = {}; + for (var _$p in module) { + if (!_starExcludes[_$p]) _setter[_$p] = module[_$p]; + } + exports(_setter); + }], + execute: function () { + + exports('internalFn', internalFn); + + function internalFn(path) { + return path[0] === '.'; + } + + } + }; +}); diff --git a/test/form/samples/export-all-before-named/_expected/umd.js b/test/form/samples/export-all-before-named/_expected/umd.js new file mode 100644 index 00000000000..8781813e164 --- /dev/null +++ b/test/form/samples/export-all-before-named/_expected/umd.js @@ -0,0 +1,23 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('external')) : + typeof define === 'function' && define.amd ? define(['exports', 'external'], factory) : + (global = global || self, factory(global.exposedInternals = {}, global.external)); +}(this, function (exports, external) { 'use strict'; + + function internalFn(path) { + return path[0] === '.'; + } + + Object.keys(external).forEach(function (key) { + Object.defineProperty(exports, key, { + enumerable: true, + get: function () { + return external[key]; + } + }); + }); + exports.internalFn = internalFn; + + Object.defineProperty(exports, '__esModule', { value: true }); + +})); diff --git a/test/form/samples/export-all-before-named/internal.js b/test/form/samples/export-all-before-named/internal.js new file mode 100644 index 00000000000..dcfc856b8c6 --- /dev/null +++ b/test/form/samples/export-all-before-named/internal.js @@ -0,0 +1,3 @@ +export function internalFn(path) { + return path[0] === '.'; +} diff --git a/test/form/samples/export-all-before-named/main.js b/test/form/samples/export-all-before-named/main.js new file mode 100644 index 00000000000..ddc3e8e07fc --- /dev/null +++ b/test/form/samples/export-all-before-named/main.js @@ -0,0 +1,2 @@ +export * from 'external'; +export * from './internal.js';