feat(code): add partial replace capability to extend method#425
feat(code): add partial replace capability to extend method#425ader-h wants to merge 1 commit intoopentiny:devfrom
Conversation
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/util/merge.js`:
- Around line 65-74: The partial-merge branch of merge is dropping updates and
leaking control metadata; update the loop so it skips the control key 'partial',
and when merging do not ignore the returned value: compute const merged =
merge(baseOption[key] ?? {}, extend[key]) (or if baseOption[key] is a primitive,
coerce to an object/container first), then assign baseOption[key] = merged; keep
the Object.hasOwnProperty.call(extend, key) guard and ensure you don't mutate
extend or copy the 'partial' flag into baseOption.
| for (const key in extend) { | ||
| if (Object.hasOwnProperty.call(extend, key)) { | ||
| baseOption[key] = extend[key]; | ||
| // 部分替换 | ||
| if (extend.partial){ | ||
| if (Object.hasOwnProperty.call(extend, key)) { | ||
| merge(baseOption[key], extend[key]) | ||
| } | ||
| } else { | ||
| if (Object.hasOwnProperty.call(extend, key)) { | ||
| baseOption[key] = extend[key]; | ||
| } |
There was a problem hiding this comment.
Fix partial-merge path: it can drop updates and mutate with control metadata.
On Line 69, merge(baseOption[key], extend[key]) ignores the returned value, so new keys / some partial updates are lost. It can also fail when baseOption[key] is primitive and extend[key] is object. Also, the loop currently processes partial itself, which can leak control metadata into baseOption.
Proposed fix
function mergeExtend(iChartOption, baseOption) {
if (!iChartOption) return;
let extend = iChartOption.extend;
if (!extend) return;
for (const key in extend) {
- // 部分替换
- if (extend.partial){
- if (Object.hasOwnProperty.call(extend, key)) {
- merge(baseOption[key], extend[key])
- }
- } else {
- if (Object.hasOwnProperty.call(extend, key)) {
- baseOption[key] = extend[key];
- }
- }
+ if (!Object.prototype.hasOwnProperty.call(extend, key) || key === 'partial') continue;
+ // 部分替换
+ if (extend.partial) {
+ if (isObject(extend[key]) && !isArray(extend[key])) {
+ const current = isObject(baseOption[key]) && !isArray(baseOption[key]) ? baseOption[key] : undefined;
+ baseOption[key] = merge(current, extend[key]);
+ } else {
+ baseOption[key] = extend[key];
+ }
+ } else {
+ baseOption[key] = extend[key];
+ }
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/util/merge.js` around lines 65 - 74, The partial-merge branch of merge is
dropping updates and leaking control metadata; update the loop so it skips the
control key 'partial', and when merging do not ignore the returned value:
compute const merged = merge(baseOption[key] ?? {}, extend[key]) (or if
baseOption[key] is a primitive, coerce to an object/container first), then
assign baseOption[key] = merged; keep the Object.hasOwnProperty.call(extend,
key) guard and ensure you don't mutate extend or copy the 'partial' flag into
baseOption.
1 participant
Summary by CodeRabbit