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: should count in module scope variables #18349

Merged
merged 1 commit into from May 7, 2024
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
85 changes: 47 additions & 38 deletions lib/javascript/JavascriptModulesPlugin.js
Expand Up @@ -822,7 +822,7 @@ class JavascriptModulesPlugin {
const lastInlinedModule = last(inlinedModules);
const startupSource = new ConcatSource();
startupSource.add(`var ${RuntimeGlobals.exports} = {};\n`);
const renamedInlinedModules = this.renamedRootModule(
const renamedInlinedModule = this.renameInlineModule(
allModules,
renderContext,
inlinedModules,
Expand All @@ -832,7 +832,7 @@ class JavascriptModulesPlugin {

for (const m of inlinedModules) {
const renderedModule =
renamedInlinedModules.get(m) ||
renamedInlinedModule.get(m) ||
this.renderModule(m, chunkRenderContext, hooks, false);

if (renderedModule) {
Expand Down Expand Up @@ -1400,7 +1400,7 @@ class JavascriptModulesPlugin {
* @param {CompilationHooks} hooks hooks
* @returns {Map<Module, Source>} renamed inlined modules
*/
renamedRootModule(
renameInlineModule(
allModules,
renderContext,
inlinedModules,
Expand All @@ -1409,14 +1409,12 @@ class JavascriptModulesPlugin {
) {
const { runtimeTemplate } = renderContext;

/** @type {Map<Module, { source: Source, ast: any, variables: Set<Variable> }>} */
/** @type {Map<Module, { source: Source, ast: any, variables: Set<Variable>, usedInNonInlined: Set<Variable>}>} */
const inlinedModulesToInfo = new Map();
/** @type {Set<string>} */
const nonInlinedModuleThroughIdentifiers = new Set();
/** @type {Map<Module, Source>} */
const renamedInlinedModules = new Map();
/** @type {Set<{ m: Module, variable: Variable}>} */
const usedIdentifiers = new Set();

for (const m of allModules) {
const isInlinedModule = inlinedModules && inlinedModules.has(m);
Expand All @@ -1443,55 +1441,66 @@ class JavascriptModulesPlugin {
const globalScope = scopeManager.acquire(ast);
if (inlinedModules && inlinedModules.has(m)) {
const moduleScope = globalScope.childScopes[0];
const variables = new Set();
for (const variable of moduleScope.variables) {
variables.add(variable);
}
inlinedModulesToInfo.set(m, { source: moduleSource, ast, variables });
inlinedModulesToInfo.set(m, {
source: moduleSource,
ast,
variables: new Set(moduleScope.variables),
usedInNonInlined: new Set()
});
} else {
for (const ref of globalScope.through) {
nonInlinedModuleThroughIdentifiers.add(ref.identifier.name);
}
}
}

for (const [module, { variables }] of inlinedModulesToInfo) {
for (const [, { variables, usedInNonInlined }] of inlinedModulesToInfo) {
for (const variable of variables) {
if (nonInlinedModuleThroughIdentifiers.has(variable.name)) {
usedIdentifiers.add({ m: module, variable });
usedInNonInlined.add(variable);
}
}
}

const usedName = new Set();
for (const { variable, m } of usedIdentifiers) {
const references = getAllReferences(variable);
const { ast, source: _source } = inlinedModulesToInfo.get(m);
for (const [m, moduleInfo] of inlinedModulesToInfo) {
const { ast, source: _source, usedInNonInlined } = moduleInfo;
const source = new ReplaceSource(_source);
const allIdentifiers = new Set(
references.map(r => r.identifier).concat(variable.identifiers)
);
if (usedInNonInlined.size === 0) {
renamedInlinedModules.set(m, source);
continue;
}

const newName = this.findNewName(
variable.name,
usedName,
m.readableIdentifier(runtimeTemplate.requestShortener)
const usedNames = new Set(
Array.from(inlinedModulesToInfo.get(m).variables).map(v => v.name)
);

for (const identifier of allIdentifiers) {
const r = identifier.range;
const path = getPathInAst(ast, identifier);
if (path && path.length > 1) {
const maybeProperty =
path[1].type === "AssignmentPattern" && path[1].left === path[0]
? path[2]
: path[1];
if (maybeProperty.type === "Property" && maybeProperty.shorthand) {
source.insert(r[1], `: ${newName}`);
continue;
for (const variable of usedInNonInlined) {
const references = getAllReferences(variable);
const allIdentifiers = new Set(
references.map(r => r.identifier).concat(variable.identifiers)
);

const newName = this.findNewName(
variable.name,
usedNames,
m.readableIdentifier(runtimeTemplate.requestShortener)
);
usedNames.add(newName);
for (const identifier of allIdentifiers) {
const r = identifier.range;
const path = getPathInAst(ast, identifier);
if (path && path.length > 1) {
const maybeProperty =
path[1].type === "AssignmentPattern" && path[1].left === path[0]
? path[2]
: path[1];
if (maybeProperty.type === "Property" && maybeProperty.shorthand) {
source.insert(r[1], `: ${newName}`);
continue;
}
}
source.replace(r[0], r[1] - 1, newName);
}
source.replace(r[0], r[1] - 1, newName);
}

renamedInlinedModules.set(m, source);
Expand Down Expand Up @@ -1524,10 +1533,10 @@ class JavascriptModulesPlugin {
}

let i = 0;
let nameWithNumber = Template.toIdentifier(`${oldName}_${i}`);
let nameWithNumber = Template.toIdentifier(`${name}_${i}`);
while (usedName.has(nameWithNumber)) {
i++;
nameWithNumber = Template.toIdentifier(`${oldName}_${i}`);
nameWithNumber = Template.toIdentifier(`${name}_${i}`);
}
return nameWithNumber;
}
Expand Down
Expand Up @@ -2,7 +2,8 @@ import { value as v1 } from "./module1";
const v2 = require("./module2")
const module3Inc = require("./module3")

var value = 42;
const index_value = 10;
let value = 42;

function inc() {
value++;
Expand All @@ -15,4 +16,5 @@ it("single inlined module should not be wrapped in IIFE", () => {
expect(module3Inc).toBe(undefined);
inc();
expect(value).toBe(43);
expect(index_value).toBe(10);
});
2 changes: 1 addition & 1 deletion types.d.ts
Expand Up @@ -5529,7 +5529,7 @@ declare class JavascriptModulesPlugin {
renderContext: RenderBootstrapContext,
hooks: CompilationHooksJavascriptModulesPlugin
): string;
renamedRootModule(
renameInlineModule(
allModules: Module[],
renderContext: MainRenderContext,
inlinedModules: Set<Module>,
Expand Down