Skip to content

Conversation

@blacksharksjc
Copy link

@blacksharksjc blacksharksjc commented Oct 2, 2025

Description 描述

问题现象

插件在合并tabbar配置时,出现丢失部分pages.config.(ts|mts|cts|js|cjs|mjs|json)中配置的tabbar属性,仅保留了list部分内容

问题示例

  1. pages.config.ts代码示例
export default defineUniPages({
    ..., // 其他配置
    tabbar: {
        color: "#999999",
        selectedColor: "#000000",
    }
})
  1. 页面中使用definePage进行配置
definePage({
  tabBar: {
    index: 0,
    text: "首页",
    iconPath: "static/images/tabbar/index.png",
    selectedIconPath: "static/images/tabbar/index-active.png",
  },
});
  1. 插件输出的pages.json
{
    "tabbar": {
         "list": [{
            "text": "首页",
            "iconPath": "static/images/tabbar/index.png",
            "selectedIconPath": "static/images/tabbar/index-active.png",
            "pagePath": "pages/index/index"
         }]
    }
}

修复结果

插件根据以下优先级合并tabbar额外属性:pages.json -> pages.config.(ts|mts|cts|js|cjs|mjs|json)。
根据上述示例配置,最终输出结果如下:

{
    "tabbar": {
         "list": [{
            "text": "首页",
            "iconPath": "static/images/tabbar/index.png",
            "selectedIconPath": "static/images/tabbar/index-active.png",
            "pagePath": "pages/index/index"
         }],
         color: "#999999",
         selectedColor: "#000000",
    }
}

Linked Issues 关联的 Issues

Additional context 额外上下文

Summary by CodeRabbit

  • Bug Fixes
    • Preserve existing top-level page properties (pages, subpackages, tab bar) when merging new configs to avoid losing prior settings.
    • Improve merging logic for pages and subpackages so existing entries are retained and combined correctly.
    • Ensure tab bar is merged in a platform-aware way, preserving customizations and explicitly clearing it when absent to prevent inconsistent behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 2, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Preserves existing top-level page JSON properties (pages, subPackages, tabBar) when merging new page config; rebuilds pageJson from non-page/tabBar fields; merges pages and subPackages with prior values; introduces platform-aware tabBar merge that can preserve, update, or clear tabBar explicitly.

Changes

Cohort / File(s) Summary
Core context changes
packages/core/src/context.ts
Preserve top-level oldPages, oldSubPackages, and oldTabBar from current pages JSON; construct pageJson from only "other" config props; feed oldPages into mergePlatformItems for pages; initialize/retain subPackages from oldSubPackages or a new CommentArray and keep per-subpackage merges; replace tabBar merging with platform-aware flow that merges lists, merges other tabBar properties, and explicitly clears pageJson.tabBar when no tabBar is produced.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Builder as Builder/Plugin
  participant Context as PageContext.mergePageMetaData
  participant Old as existing pages JSON
  participant Merge as mergePlatformItems / tabBarMerge
  participant PageJson as new pageJson

  Builder->>Context: provide new page config
  Context->>Old: read top-level `oldPages`, `oldSubPackages`, `oldTabBar`
  Context->>PageJson: build base from config "others" (no pages/tabBar)
  Context->>Merge: merge pages using `oldPages` + new pages -> mergedPages
  Merge-->>PageJson: assign mergedPages
  Context->>Merge: for subPackages, use `oldSubPackages` or new CommentArray, then per-subpkg merge
  Merge-->>PageJson: assign mergedSubPackages

  Context->>Merge: merge tabBar -> { list, ...tabBarOthers }
  alt merged tabBar produced
    Merge-->>PageJson: apply merged `list` (platform-aware merge)
    Context->>PageJson: copy `tabBarOthers` into pageJson.tabBar (merge/preserve)
  else no tabBar produced
    Context->>PageJson: set pageJson.tabBar = undefined  %% explicit clear for unsupported platform cases
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

size/XS

Poem

A rabbit nibbles through the trees of JSON,
Keeps old routes safe and trims what’s gone,
Merges lists with gentle care,
Clears empty paths with tidy flair,
Hooray — a hop, a patch, and on! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed This title clearly states the bug fix for tabBar property merging from pages.config.ts into pages.json and uses the conventional ‘fix:’ prefix. It identifies the affected files and describes the issue succinctly so reviewers can immediately understand the core change. The phrasing is specific and avoids unnecessary detail, meeting the PR title guidelines.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d569ed and 9dfd5ad.

📒 Files selected for processing (1)
  • packages/core/src/context.ts (2 hunks)

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/core/src/context.ts (1)

490-496: LGTM! The tabBar merge logic correctly preserves existing properties.

The implementation correctly gives priority to properties already in pageJson.tabBar, only copying additional properties from the merged tabBar when they don't exist. This aligns with the stated priority of pages.json → pages.config.

Consider improving type safety by reducing the use of as any:

-      const { list: __, ...restTabBarProps } = tabBar
-      Object.keys(restTabBarProps).forEach((key) => {
-        if (!(key in (pageJson.tabBar as any))) {
-          (pageJson.tabBar as any)[key] = restTabBarProps[key]
+      const { list: __, ...restTabBarProps } = tabBar
+      Object.keys(restTabBarProps).forEach((key) => {
+        const tabBarObj = pageJson.tabBar as Record<string, any>
+        if (!(key in tabBarObj)) {
+          tabBarObj[key] = restTabBarProps[key as keyof typeof restTabBarProps]
         }
       })

This consolidates the type assertions and makes the intent clearer.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b71986a and 4d569ed.

📒 Files selected for processing (1)
  • packages/core/src/context.ts (1 hunks)

@edwinhuish
Copy link
Collaborator

edwinhuish commented Oct 9, 2025

谢谢你的PR, 确实是漏了。但是看了一下你的实现,感觉可以优化一下:

    // tabbar
    const { list, ...tabBarOthers } = (await this.getTabBarMerged()) || {}
    if (list) {
      const { list: oldList } = (pageJson as any)?.tabBar || {}
      const newList = mergePlatformItems(oldList, currentPlatform, list, 'pagePath');
      (pageJson as any).tabBar = {
        ...tabBarOthers, // 每次都直接更新除 list 外的其他属性
        list: newList,
      }
    }
    else {
      (pageJson as any).tabBar = undefined // 直接清空,暂不支持 A 平台有 tabBar, B 平台无 tabBar 的情况。
    }

@pull-request-size pull-request-size bot added size/M and removed size/XS labels Oct 9, 2025
@edwinhuish edwinhuish merged commit defce70 into uni-helper:main Oct 9, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants