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

Prevent unneeded exports when entry facades are created and ensure all exported variables in facades are imported #3590

Merged
merged 2 commits into from May 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
97 changes: 52 additions & 45 deletions src/Chunk.ts
Expand Up @@ -136,6 +136,7 @@ export default class Chunk {
) {
chunk.dependencies.add(facadedModule.chunk!);
}
chunk.ensureReexportsAreAvailableForModule(facadedModule);
chunk.facadeModule = facadedModule;
chunk.strictFacade = true;
return chunk;
Expand Down Expand Up @@ -214,7 +215,7 @@ export default class Chunk {
}
}

canModuleBeFacade(module: Module, exposedNamespaces: NamespaceVariable[]): boolean {
canModuleBeFacade(module: Module, exposedVariables: Set<Variable>): boolean {
const moduleExportNamesByVariable = module.getExportNamesByVariable();
for (const exposedVariable of this.exports) {
if (!moduleExportNamesByVariable.has(exposedVariable)) {
Expand All @@ -236,7 +237,7 @@ export default class Chunk {
return false;
}
}
for (const exposedVariable of exposedNamespaces) {
for (const exposedVariable of exposedVariables) {
if (
!(moduleExportNamesByVariable.has(exposedVariable) || exposedVariable.module === module)
) {
Expand Down Expand Up @@ -279,7 +280,14 @@ export default class Chunk {
module.includedDynamicImporters.some(importingModule => importingModule.chunk !== this)
);
this.isDynamicEntry = dynamicEntryModules.length > 0;
const exposedNamespaces = dynamicEntryModules.map(module => module.namespace);
const exposedVariables = new Set<Variable>(dynamicEntryModules.map(module => module.namespace));
for (const module of this.entryModules) {
if (module.preserveSignature) {
for (const exportedVariable of module.getExportNamesByVariable().keys()) {
exposedVariables.add(exportedVariable);
}
}
}
for (const module of this.entryModules) {
const requiredFacades: FacadeName[] = [...module.userChunkNames].map(name => ({
name
Expand All @@ -295,11 +303,14 @@ export default class Chunk {
!this.facadeModule &&
(this.graph.preserveModules ||
module.preserveSignature !== 'strict' ||
this.canModuleBeFacade(module, exposedNamespaces))
this.canModuleBeFacade(module, exposedVariables))
) {
this.facadeModule = module;
module.facadeChunk = this;
this.strictFacade = module.preserveSignature === 'strict';
if (module.preserveSignature) {
this.strictFacade = module.preserveSignature === 'strict';
this.ensureReexportsAreAvailableForModule(module);
}
this.assignFacadeName(requiredFacades.shift()!, module);
}

Expand All @@ -308,15 +319,15 @@ export default class Chunk {
}
}
for (const module of dynamicEntryModules) {
if (!this.facadeModule && this.canModuleBeFacade(module, exposedNamespaces)) {
if (!this.facadeModule && this.canModuleBeFacade(module, exposedVariables)) {
this.facadeModule = module;
module.facadeChunk = this;
this.strictFacade = true;
this.assignFacadeName({}, module);
} else if (
this.facadeModule === module &&
!this.strictFacade &&
this.canModuleBeFacade(module, exposedNamespaces)
this.canModuleBeFacade(module, exposedVariables)
) {
this.strictFacade = true;
} else if (!(module.facadeChunk && module.facadeChunk.strictFacade)) {
Expand Down Expand Up @@ -749,6 +760,28 @@ export default class Chunk {
return hash.digest('hex').substr(0, 8);
}

private ensureReexportsAreAvailableForModule(module: Module) {
const map = module.getExportNamesByVariable();
for (const exportedVariable of map.keys()) {
const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
const importedVariable = isSynthetic
? (exportedVariable as SyntheticNamedExportVariable).getBaseVariable()
: exportedVariable;
const exportingModule = importedVariable.module;
if (
exportingModule &&
exportingModule.chunk &&
exportingModule.chunk !== this &&
!(importedVariable instanceof NamespaceVariable && this.graph.preserveModules)
) {
exportingModule.chunk.exports.add(importedVariable);
if (isSynthetic) {
this.imports.add(importedVariable);
}
}
}
}

private finaliseDynamicImports(options: OutputOptions) {
const stripKnownJsExtensions = options.format === 'amd';
for (const [module, code] of this.renderedModuleSources) {
Expand Down Expand Up @@ -1067,11 +1100,8 @@ export default class Chunk {
if (variable instanceof ExportDefaultVariable) {
variable = variable.getOriginalVariable();
}
while (variable instanceof SyntheticNamedExportVariable) {
if (variable instanceof SyntheticNamedExportVariable) {
variable = variable.getBaseVariable();
if (variable instanceof ExportDefaultVariable) {
variable = variable.getOriginalVariable();
}
}
if (variable.module && (variable.module as Module).chunk !== this) {
this.imports.add(variable);
Expand All @@ -1085,44 +1115,21 @@ export default class Chunk {
}
if (
(module.isEntryPoint && module.preserveSignature !== false) ||
module.includedDynamicImporters.some(importer => importer.chunk !== this)
module.includedDynamicImporters.some(importer => importer.chunk !== this) ||
module.namespace.included
) {
const map = module.getExportNamesByVariable();
for (const exportedVariable of map.keys()) {
if (module.isEntryPoint && module.preserveSignature !== false) {
this.exports.add(exportedVariable);
}
const isSynthetic = exportedVariable instanceof SyntheticNamedExportVariable;
const importedVariable = isSynthetic
? (exportedVariable as SyntheticNamedExportVariable).getBaseVariable()
: exportedVariable;
const exportingModule = importedVariable.module;
if (
exportingModule &&
exportingModule.chunk &&
exportingModule.chunk !== this &&
!(importedVariable instanceof NamespaceVariable && this.graph.preserveModules)
) {
exportingModule.chunk.exports.add(importedVariable);
if (isSynthetic) {
this.imports.add(importedVariable);
}
}
}
}
if (module.namespace.included) {
for (const reexportName of Object.keys(module.reexportDescriptions)) {
const reexport = module.reexportDescriptions[reexportName];
const variable = reexport.module.getVariableForExportName(reexport.localName);
if ((variable.module as Module).chunk && (variable.module as Module).chunk !== this) {
this.imports.add(variable);
(variable.module as Module).chunk!.exports.add(variable);
}
}
this.ensureReexportsAreAvailableForModule(module);
}
for (const { node, resolution } of module.dynamicImports) {
if (node.included && resolution instanceof Module && resolution.chunk === this)
if (
node.included &&
resolution instanceof Module &&
resolution.chunk === this &&
!resolution.namespace.included
) {
resolution.namespace.include();
this.ensureReexportsAreAvailableForModule(resolution);
}
}
}
}
12 changes: 9 additions & 3 deletions src/ast/variables/SyntheticNamedExportVariable.ts
@@ -1,4 +1,5 @@
import Module, { AstContext } from '../../Module';
import ExportDefaultVariable from './ExportDefaultVariable';
import Variable from './Variable';

export default class SyntheticNamedExportVariable extends Variable {
Expand All @@ -14,9 +15,14 @@ export default class SyntheticNamedExportVariable extends Variable {
}

getBaseVariable(): Variable {
return this.defaultVariable instanceof SyntheticNamedExportVariable
? this.defaultVariable.getBaseVariable()
: this.defaultVariable;
let baseVariable = this.defaultVariable;
if (baseVariable instanceof ExportDefaultVariable) {
baseVariable = baseVariable.getOriginalVariable();
}
if (baseVariable instanceof SyntheticNamedExportVariable) {
baseVariable = baseVariable.getBaseVariable();
}
return baseVariable;
}

getName(): string {
Expand Down
7 changes: 7 additions & 0 deletions test/chunking-form/samples/circular-entry-points2/_config.js
@@ -0,0 +1,7 @@
module.exports = {
description: 'does not create a facade for one circular entry point if possible',
expectedWarnings: ['CIRCULAR_DEPENDENCY'],
options: {
input: ['main1.js', 'main2.js']
}
};
@@ -0,0 +1,9 @@
define(['exports', './main2'], function (exports, main2) { 'use strict';



exports.p = main2.p2;

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

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

class C {
fn (num) {
console.log(num - p$1);
}
}

var p = 43;

new C().fn(p);

class C$1 {
fn (num) {
console.log(num - p);
}
}

var p$1 = 42;

new C$1().fn(p$1);

exports.p = p;
exports.p2 = p$1;

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

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

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

var main2 = require('./main2.js');



exports.p = main2.p2;
@@ -0,0 +1,26 @@
'use strict';

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

class C {
fn (num) {
console.log(num - p$1);
}
}

var p = 43;

new C().fn(p);

class C$1 {
fn (num) {
console.log(num - p);
}
}

var p$1 = 42;

new C$1().fn(p$1);

exports.p = p;
exports.p2 = p$1;
@@ -0,0 +1 @@
export { p2 as p } from './main2.js';
@@ -0,0 +1,21 @@
class C {
fn (num) {
console.log(num - p$1);
}
}

var p = 43;

new C().fn(p);

class C$1 {
fn (num) {
console.log(num - p);
}
}

var p$1 = 42;

new C$1().fn(p$1);

export { p, p$1 as p2 };
@@ -0,0 +1,13 @@
System.register(['./main2.js'], function (exports) {
'use strict';
return {
setters: [function (module) {
exports('p', module.p2);
}],
execute: function () {



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

class C {
fn (num) {
console.log(num - p$1);
}
}

var p = exports('p', 43);

new C().fn(p);

class C$1 {
fn (num) {
console.log(num - p);
}
}

var p$1 = exports('p2', 42);

new C$1().fn(p$1);

}
};
});
7 changes: 7 additions & 0 deletions test/chunking-form/samples/circular-entry-points2/dep1.js
@@ -0,0 +1,7 @@
import { p } from './main2.js';

export class C {
fn (num) {
console.log(num - p);
}
}
7 changes: 7 additions & 0 deletions test/chunking-form/samples/circular-entry-points2/dep2.js
@@ -0,0 +1,7 @@
import { p } from './main1.js';

export class C {
fn (num) {
console.log(num - p);
}
}
4 changes: 4 additions & 0 deletions test/chunking-form/samples/circular-entry-points2/main1.js
@@ -0,0 +1,4 @@
import { C } from './dep1.js';
export var p = 42;

new C().fn(p);
5 changes: 5 additions & 0 deletions test/chunking-form/samples/circular-entry-points2/main2.js
@@ -0,0 +1,5 @@
import { C } from './dep2.js';
export var p = 43;
export {p as p2} from './main1'

new C().fn(p);
8 changes: 8 additions & 0 deletions test/chunking-form/samples/circular-entry-points3/_config.js
@@ -0,0 +1,8 @@
module.exports = {
description:
'creates facades for all circular entry points if they become tainted by another entry',
expectedWarnings: ['CIRCULAR_DEPENDENCY'],
options: {
input: ['main1.js', 'main2.js', 'main3.js']
}
};