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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import external from 'external';
import external$1 from 'external';



export default external;
export default external$1;
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
description: 'confirm exports are deconflicted when exporting nested index aliases',
options: {
input: 'main.js',
preserveModules: true
}
};
Original file line number Diff line number Diff line change
@@ -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 });

});
Original file line number Diff line number Diff line change
@@ -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 });

});
Original file line number Diff line number Diff line change
@@ -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 });

});
Original file line number Diff line number Diff line change
@@ -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;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

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

const TEST_MODULE_A = 'A';

exports.TEST_MODULE_A = TEST_MODULE_A;
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict';

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

const TEST_MODULE_B = 'A';

exports.TEST_MODULE_B = TEST_MODULE_B;
Original file line number Diff line number Diff line change
@@ -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 };
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const TEST_MODULE_A = 'A';

export { TEST_MODULE_A };
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const TEST_MODULE_B = 'A';

export { TEST_MODULE_B };
Original file line number Diff line number Diff line change
@@ -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 () {



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

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

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

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

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

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

export { ModuleB_V1 }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const TEST_MODULE_B = 'A'
Original file line number Diff line number Diff line change
@@ -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);
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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