improve merging of resolve and parsing options #9126
Conversation
by rules and via loader API Objects are deeply merged Arrays overwrite by default, but can reference old value with "..." fixes #8968
|
For maintainers only:
|
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
|
I've created an issue to document this in webpack/webpack.js.org. |
| newObject[key] = second[key]; | ||
| continue; | ||
| } | ||
| const secondValue = second[key]; |
There was a problem hiding this comment.
This assignment should be before the if statement above. Then replace second[key] in the body of the if with secondValue.
There was a problem hiding this comment.
Can you open issue and describe problem?
| } | ||
| const secondValue = second[key]; | ||
| if (typeof secondValue !== "object" || secondValue === null) { | ||
| newObject[key] = secondValue; |
There was a problem hiding this comment.
There should probably be continue here. If secondValue is not an object or null, then it will not be an Array and firstValue will equal secondValue in the else portion of the if below resulting in reassignment of newObject[key] to secondValue. The assignment of newObject[key] = secondValue currently occurs 4 times in this code. A cleaner approach might be to either introduce a variable called newValue which is initially set to secondValue, modified if needed, and then newObject[key] assigned to newValue at the end of the loop, OR negate the above if statement, wrap it around the one below, move this assignment to the end of the loop and add a continue after all other places where newObject[key] is set.
| newObject[key] = secondValue; | ||
| } | ||
| if (Array.isArray(secondValue)) { | ||
| const firstValue = newObject[key]; |
There was a problem hiding this comment.
This line is duplicated in the else portion of this if statement. The firstValue variable should thus be declared before the if statement.
| } | ||
| newObject[key] = newArray; | ||
| } else { | ||
| newObject[key] = secondValue; |
| ) { | ||
| newObject[key] = cleverMerge(firstValue, secondValue); | ||
| } else { | ||
| newObject[key] = secondValue; |
|
@deAtog please don't write in merged PRs/closed issues, it is hard to track because we have many issues/PRs in many repos, so it can be wasted, better open issue and describe problem, thanks |
improve merging of resolve and parsing options
by rules and via loader API
Objects are deeply merged
Arrays overwrite by default, but can reference old value with "..."
fixes #8968
What kind of change does this PR introduce?
feature, maybe bugfix
Did you add tests for your changes?
yes
Does this PR introduce a breaking change?
yes
What needs to be documented once your changes are merged?
Rule.resolve/Rule.parser objects are now deeply merged.
In array the special
"..."item can now be used to reference to old value, while otherwise array values overwrite each other.This also affects the
getResolve(options)API in the loader API