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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize property accessors for es6 namespaces and chained member/call expressions #17203

Merged
merged 25 commits into from May 31, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4a0b9a0
property accessors, create failing test
bworline May 16, 2023
44d1653
hack to get test to pass; must be deleted
bworline May 16, 2023
457b2a8
remove extra code from hack
bworline May 16, 2023
6a9f79b
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 21, 2023
6f43ce3
checkpoint
bworline May 23, 2023
f041e8d
remove extra memberRangeStarts changes
bworline May 23, 2023
852961f
add comment
bworline May 23, 2023
f59c12d
update comment
bworline May 23, 2023
afde59f
add and handle another test case
bworline May 24, 2023
7bdd643
move comment
bworline May 24, 2023
a8bb2d7
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 24, 2023
5022bf9
yarn special-lint-fix
bworline May 24, 2023
5b88a48
yarn type-lint
bworline May 24, 2023
af66269
fix use of "at"
bworline May 24, 2023
b40c05e
add test filter
bworline May 24, 2023
f3fcd69
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 24, 2023
52912c0
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 24, 2023
3210786
update type.d.ts
bworline May 24, 2023
43fdf61
update types.d.ts
bworline May 24, 2023
b834303
add more error handling and json import support
bworline May 26, 2023
9abdfec
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 27, 2023
9bb13a8
incorporate CR feedback
bworline May 31, 2023
34eba0d
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 31, 2023
5ab7e50
handle null dep.idRangeStarts
bworline May 31, 2023
77c4deb
Merge branch 'main' of https://github.com/webpack/webpack into ns
bworline May 31, 2023
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
Expand Up @@ -35,7 +35,7 @@ class HarmonyEvaluatedImportSpecifierDependency extends HarmonyImportSpecifierDe
* @param {string} operator operator
*/
constructor(request, sourceOrder, ids, name, range, assertions, operator) {
super(request, sourceOrder, ids, name, range, false, assertions);
super(request, sourceOrder, ids, name, range, false, assertions, []);
this.operator = operator;
}

Expand Down
23 changes: 18 additions & 5 deletions lib/dependencies/HarmonyImportDependencyParserPlugin.js
Expand Up @@ -195,7 +195,8 @@ module.exports = class HarmonyImportDependencyParserPlugin {
settings.name,
expr.range,
exportPresenceMode,
settings.assertions
settings.assertions,
[]
);
dep.referencedPropertiesInDestructuring =
parser.destructuringAssignmentPropertiesFor(expr);
Expand All @@ -211,14 +212,19 @@ module.exports = class HarmonyImportDependencyParserPlugin {
.for(harmonySpecifierTag)
.tap(
"HarmonyImportDependencyParserPlugin",
(expression, members, membersOptionals) => {
(expression, members, membersOptionals, memberRangeStarts) => {
const settings = /** @type {HarmonySettings} */ (
parser.currentTagData
);
const nonOptionalMembers = getNonOptionalPart(
members,
membersOptionals
);
const rangeStarts = memberRangeStarts.slice(
0,
memberRangeStarts.length -
(members.length - nonOptionalMembers.length)
);
const expr =
nonOptionalMembers !== members
? getNonOptionalMemberChain(
Expand All @@ -234,7 +240,8 @@ module.exports = class HarmonyImportDependencyParserPlugin {
settings.name,
expr.range,
exportPresenceMode,
settings.assertions
settings.assertions,
rangeStarts
);
dep.referencedPropertiesInDestructuring =
parser.destructuringAssignmentPropertiesFor(expr);
Expand All @@ -249,7 +256,7 @@ module.exports = class HarmonyImportDependencyParserPlugin {
.for(harmonySpecifierTag)
.tap(
"HarmonyImportDependencyParserPlugin",
(expression, members, membersOptionals) => {
(expression, members, membersOptionals, memberRangeStarts) => {
const { arguments: args, callee } = expression;
const settings = /** @type {HarmonySettings} */ (
parser.currentTagData
Expand All @@ -258,6 +265,11 @@ module.exports = class HarmonyImportDependencyParserPlugin {
members,
membersOptionals
);
const rangeStarts = memberRangeStarts.slice(
0,
memberRangeStarts.length -
(members.length - nonOptionalMembers.length)
);
const expr =
nonOptionalMembers !== members
? getNonOptionalMemberChain(
Expand All @@ -273,7 +285,8 @@ module.exports = class HarmonyImportDependencyParserPlugin {
settings.name,
expr.range,
exportPresenceMode,
settings.assertions
settings.assertions,
rangeStarts
);
dep.directImport = members.length === 0;
dep.call = true;
Expand Down
78 changes: 72 additions & 6 deletions lib/dependencies/HarmonyImportSpecifierDependency.js
Expand Up @@ -43,6 +43,7 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
* @param {Range} range range
* @param {TODO} exportPresenceMode export presence mode
* @param {Assertions=} assertions assertions
* @param {number[]=} idRangeStarts range starts for members of ids; the two arrays are right-aligned
*/
constructor(
request,
Expand All @@ -51,12 +52,14 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
name,
range,
exportPresenceMode,
assertions
assertions,
idRangeStarts // TODO webpack 6 make this non-optional. It must always be set to properly trim ids.
) {
super(request, sourceOrder, assertions);
this.ids = ids;
this.name = name;
this.range = range;
this.idRangeStarts = idRangeStarts;
this.exportPresenceMode = exportPresenceMode;
this.namespaceObjectAsContext = false;
this.call = undefined;
Expand Down Expand Up @@ -258,6 +261,7 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
write(this.ids);
write(this.name);
write(this.range);
write(this.idRangeStarts);
write(this.exportPresenceMode);
write(this.namespaceObjectAsContext);
write(this.call);
Expand All @@ -277,6 +281,7 @@ class HarmonyImportSpecifierDependency extends HarmonyImportDependency {
this.ids = read();
this.name = read();
this.range = read();
this.idRangeStarts = read();
this.exportPresenceMode = read();
this.namespaceObjectAsContext = read();
this.call = read();
Expand Down Expand Up @@ -310,14 +315,75 @@ HarmonyImportSpecifierDependency.Template = class HarmonyImportSpecifierDependen
// Skip rendering depending when dependency is conditional
if (connection && !connection.isTargetActive(runtime)) return;

const ids = dep.getIds(moduleGraph);
const exportExpr = this._getCodeForIds(dep, source, templateContext, ids);
const range = dep.range;
const ids = dep.getIds(moduleGraph); // determine minimal set of IDs.
let trimmedIds = this._trimIdsToThoseImported(ids, moduleGraph, dep);

let [rangeStart, rangeEnd] = dep.range;
if (trimmedIds.length !== ids.length) {
// The array returned from dep.idRangeStarts is right-aligned with the array returned from dep.getIds.
// Meaning, the two arrays may not always have the same number of elements, but the last element of
// dep.idRangeStarts corresponds to [the starting range position of] the last element of dep.getIds.
// Use this to find the correct range end position based on the number of ids that were trimmed.
const idx = dep.idRangeStarts.length + (trimmedIds.length - ids.length);
if (idx < 0 || idx >= dep.idRangeStarts.length) {
// cspell:ignore minifiers
// Should not happen but we can't throw an error here because of backward compatibility with
// external plugins in wp5. Instead, we just disable trimming for now. This may break some minifiers.
trimmedIds = ids;
// TODO webpack 6 remove the "trimmedIds = ids" above and uncomment the following line instead.
// throw new Error("Missing range starts data for id replacement trimming.");
} else {
rangeEnd = dep.idRangeStarts[idx];
}
bworline marked this conversation as resolved.
Show resolved Hide resolved
}

const exportExpr = this._getCodeForIds(
dep,
source,
templateContext,
trimmedIds
);
if (dep.shorthand) {
source.insert(range[1], `: ${exportExpr}`);
source.insert(rangeEnd, `: ${exportExpr}`);
} else {
source.replace(range[0], range[1] - 1, exportExpr);
source.replace(rangeStart, rangeEnd - 1, exportExpr);
}
}

/**
* @summary Determine which IDs in the id chain are actually referring to namespaces or imports,
* and which are deeper member accessors on the imported object. Only the former should be re-rendered.
* @param {string[]} ids ids
* @param {ModuleGraph} moduleGraph moduleGraph
* @param {HarmonyImportSpecifierDependency} dependency dependency
* @returns {string[]} generated code
*/
_trimIdsToThoseImported(ids, moduleGraph, dependency) {
let trimmedIds = [];
const exportsInfo = moduleGraph.getExportsInfo(
moduleGraph.getModule(dependency)
);
let currentExportsInfo = /** @type {ExportsInfo=} */ exportsInfo;
for (let i = 0; i < ids.length; i++) {
if (i === 0 && ids[i] === "default") {
continue; // ExportInfo for the next level under default is still at the root ExportsInfo, so don't advance currentExportsInfo
}
const exportInfo = currentExportsInfo.getExportInfo(ids[i]);
if (exportInfo.provided === false) {
// json imports have nested ExportInfo for elements that things that are not actually exported, so check .provided
trimmedIds = ids.slice(0, i);
break;
}
const nestedInfo = exportInfo.getNestedExportsInfo();
if (!nestedInfo) {
// once all nested exports are traversed, the next item is the actual import so stop there
trimmedIds = ids.slice(0, i + 1);
break;
}
currentExportsInfo = nestedInfo;
}
// Never trim to nothing. This can happen for invalid imports (e.g. import { notThere } from "./module", or import { anything } from "./missingModule")
return trimmedIds.length ? trimmedIds : ids;
}

/**
Expand Down
12 changes: 11 additions & 1 deletion lib/javascript/BasicEvaluatedExpression.js
Expand Up @@ -70,6 +70,8 @@ class BasicEvaluatedExpression {
this.getMembers = undefined;
/** @type {() => boolean[]} */
this.getMembersOptionals = undefined;
/** @type {() => number[]} */
this.getMemberRangeStarts = undefined;
/** @type {EsTreeNode} */
this.expression = undefined;
}
Expand Down Expand Up @@ -384,14 +386,22 @@ class BasicEvaluatedExpression {
* @param {string | VariableInfoInterface} rootInfo root info
* @param {() => string[]} getMembers members
* @param {() => boolean[]=} getMembersOptionals optional members
* @param {() => number[]=} getMemberRangeStarts range start of progressively increasing sub-expressions
* @returns {this} this
*/
setIdentifier(identifier, rootInfo, getMembers, getMembersOptionals) {
setIdentifier(
identifier,
rootInfo,
getMembers,
getMembersOptionals,
getMemberRangeStarts
) {
this.type = TypeIdentifier;
this.identifier = identifier;
this.rootInfo = rootInfo;
this.getMembers = getMembers;
this.getMembersOptionals = getMembersOptionals;
this.getMemberRangeStarts = getMemberRangeStarts;
this.sideEffects = true;
return this;
}
Expand Down