Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(deconflictChunk): Deconflict multiple index imports for ES format using nested export star statements #3435

Merged
25 changes: 24 additions & 1 deletion src/utils/deconflictChunk.ts
Expand Up @@ -18,7 +18,7 @@ const DECONFLICT_IMPORTED_VARIABLES_BY_FORMAT: {
cjs: deconflictImportsOther,
es: deconflictImportsEsm,
iife: deconflictImportsOther,
system: deconflictImportsEsm,
system: deconflictImportsEsmOrSystem,
umd: deconflictImportsOther
};

Expand Down Expand Up @@ -49,6 +49,29 @@ export function deconflictChunk(
}

function deconflictImportsEsm(
usedNames: Set<string>,
imports: Set<Variable>,
dependencies: Set<ExternalModule | Chunk>,
interop: boolean,
preserveModules: boolean
) {
// Deconflict re-exported variables of dependencies when preserveModules is true.
// However, this implementation will result in unnecessary variable renaming without
// a deeper, wider fix.
//
// TODO: https://github.com/rollup/rollup/pull/3435#discussion_r390792792
if (preserveModules) {
for (const chunkOrExternalModule of dependencies) {
chunkOrExternalModule.variableName = getSafeName(
chunkOrExternalModule.variableName,
usedNames
);
}
}
deconflictImportsEsmOrSystem(usedNames, imports, dependencies, interop);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, and easy to get back to for a deeper fix 👍


function deconflictImportsEsmOrSystem(
usedNames: Set<string>,
imports: Set<Variable>,
_dependencies: Set<ExternalModule | Chunk>,
Expand Down
@@ -1,5 +1,5 @@
import external from 'external';
import external$1 from 'external';



export default external;
export default external$1;
@@ -1,5 +1,5 @@
import { __moduleExports as other } from '../other.js';
import { __moduleExports as other$1 } from '../other.js';



export default other;
export default other$1;
@@ -1,10 +1,10 @@
import 'external';
import external from './_virtual/_external_commonjs-external';
import external$1 from './_virtual/_external_commonjs-external';
import require$$0 from './_virtual/other.js_commonjs-proxy';

const { value } = require$$0;

console.log(external, value);
console.log(external$1, value);

var commonjs = 42;

Expand Down
@@ -1,4 +1,4 @@
import external from 'external';
import external$1 from 'external';
import value from './commonjs.js';

console.log(value, external);
console.log(value, external$1);
@@ -0,0 +1,7 @@
module.exports = {
description: 'confirm exports are deconflicted when exporting nested index aliases',
options: {
input: 'main.js',
preserveModules: true
}
};
@@ -0,0 +1,10 @@
define(['exports', './module-a/v1/index', './module-b/v1/index'], function (exports, index, index$1) { 'use strict';



exports.ModuleA_V1 = index;
exports.ModuleB_V1 = index$1;

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

});
@@ -0,0 +1,9 @@
define(['exports'], function (exports) { 'use strict';

const TEST_MODULE_A = 'A';

exports.TEST_MODULE_A = TEST_MODULE_A;

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

});
@@ -0,0 +1,9 @@
define(['exports'], function (exports) { 'use strict';

const TEST_MODULE_B = 'A';

exports.TEST_MODULE_B = TEST_MODULE_B;

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

});
@@ -0,0 +1,11 @@
'use strict';

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

var index = require('./module-a/v1/index.js');
var index$1 = require('./module-b/v1/index.js');



exports.ModuleA_V1 = index;
exports.ModuleB_V1 = index$1;
@@ -0,0 +1,7 @@
'use strict';

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

const TEST_MODULE_A = 'A';

exports.TEST_MODULE_A = TEST_MODULE_A;
@@ -0,0 +1,7 @@
'use strict';

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

const TEST_MODULE_B = 'A';

exports.TEST_MODULE_B = TEST_MODULE_B;
@@ -0,0 +1,4 @@
import * as index from './module-a/v1/index.js';
export { index as ModuleA_V1 };
import * as index$1 from './module-b/v1/index.js';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main goal of this PR, to deconflict the index imports.

export { index$1 as ModuleB_V1 };
@@ -0,0 +1,3 @@
const TEST_MODULE_A = 'A';

export { TEST_MODULE_A };
@@ -0,0 +1,3 @@
const TEST_MODULE_B = 'A';

export { TEST_MODULE_B };
@@ -0,0 +1,15 @@
System.register(['./module-a/v1/index.js', './module-b/v1/index.js'], function (exports) {
'use strict';
return {
setters: [function (module) {
exports('ModuleA_V1', module);
}, function (module) {
exports('ModuleB_V1', module);
}],
execute: function () {



}
};
});
@@ -0,0 +1,10 @@
System.register([], function (exports) {
'use strict';
return {
execute: function () {

const TEST_MODULE_A = exports('TEST_MODULE_A', 'A');

}
};
});
@@ -0,0 +1,10 @@
System.register([], function (exports) {
'use strict';
return {
execute: function () {

const TEST_MODULE_B = exports('TEST_MODULE_B', 'A');

}
};
});
@@ -0,0 +1,2 @@
export * from './module-a/index'
export * from './module-b/index'
@@ -0,0 +1,3 @@
import * as ModuleA_V1 from './v1/index'

export { ModuleA_V1 }
@@ -0,0 +1 @@
export const TEST_MODULE_A = 'A'
@@ -0,0 +1,3 @@
import * as ModuleB_V1 from './v1/index'

export { ModuleB_V1 }
@@ -0,0 +1 @@
export const TEST_MODULE_B = 'A'
@@ -1,4 +1,4 @@
import { a } from './a.js';
import { a as a$1 } from './a.js';
import { d } from './one.js';

console.log(a + d);
console.log(a$1 + d);
@@ -1,5 +1,5 @@
import { b } from './b.js';
import { b as b$1 } from './b.js';

const d = b + 4;
const d = b$1 + 4;

export { d };
6 changes: 3 additions & 3 deletions test/misc/bundle-information.js
Expand Up @@ -381,9 +381,9 @@ describe('The bundle object', () => {
assert.deepEqual(
output.map(chunk => chunk.code),
[
`import { other } from './other';
`import { other as other$1 } from './other';

console.log(other);Promise.all([import('./dynamic1'), import('./dynamic2')]).then(([{dynamic1}, {dynamic2}]) => console.log(dynamic1, dynamic2));\n`,
console.log(other$1);Promise.all([import('./dynamic1'), import('./dynamic2')]).then(([{dynamic1}, {dynamic2}]) => console.log(dynamic1, dynamic2));\n`,
'const dynamic1 = "dynamic1";\n\nexport { dynamic1 };\n',
'const other = "other";\n\nexport { other };\n',
'const dynamic2 = "dynamic2";\n\nexport { dynamic2 };\n'
Expand Down Expand Up @@ -418,7 +418,7 @@ console.log(other);Promise.all([import('./dynamic1'), import('./dynamic2')]).the
originalLength: 169,
removedExports: [],
renderedExports: [],
renderedLength: 141
renderedLength: 143
}
},
{
Expand Down