fix: ensure only string type ids are registered to metaHashMap

[chilingling]

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

• The commit message follows our Commit Message Guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

• Bugfix
• Feature
• Code style update (formatting, local variables)
• Refactoring (no functional changes, no api changes)
• Build related changes
• CI related changes
• Documentation content changes
• Other... Please describe:

Background and solution

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

• Yes
• No

Other information

Summary by CodeRabbit

• Documentation
  • Expanded v2.7 upgrade guide with guidance on first-level-only registry declarations, including valid/invalid examples, scope rules, and migration steps (e.g., Materials plugin).
  • Added documentation for registry hotfix capability to patch official plugins in emergencies, with usage examples and link to advanced docs.
• Bug Fixes
  • Improved validation of registry IDs to accept only string identifiers, preventing accidental registration of invalid entries and reducing unexpected behavior.

[coderabbitai]

Walkthrough

Updates v2.7 upgrade guide with two sections on first-level-only registry declarations and hotfix/patching. Tightens genDefaultHashMap in packages/register/src/common.ts to only register entries whose id is a non-empty string and to ignore nested declarations; no public API signature changes.

Changes

Cohort / File(s)	Summary
Docs: v2.7 upgrade guide additions docs/changelog/v2.7-upgrade-guide.md	Adds sections: (1) registry reads only first-level keys; nested declarations/metas are ignored; includes invalid/valid examples, scope rules, and migration notes (incl. Materials plugin). (2) documents hotfix capability to patch official plugins/functions/templates with example and link to advanced registry docs.
Registry traversal and id check packages/register/src/common.ts	In genDefaultHashMap, now requires id to be a string (and not 'metaData') before adding to metaHashMap/registryApiAndOptionsMap; preserves recursive traversal but effectively ignores non-string ids and nested metas. No API surface change.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant Registry as Registry Object
  participant Generator as genDefaultHashMap
  participant Maps as metaHashMap/registryApiAndOptionsMap

  App->>Generator: Generate default hash maps from Registry
  loop Traverse first-level keys
    Generator->>Registry: Read key (id), value
    alt id is string and id != "metaData"
      Generator->>Maps: set(id, value)
    else id missing/non-string or "metaData"
      note right of Generator: Skip registration
    end
  end
  note over Generator,Maps: Nested declarations are ignored in v2.7

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I hop through maps with careful sight,
First-level fields now shine so bright.
Strings for ids, the rule we keep,
While nested burrows fall asleep.
A hotfix nibble, swift and neat—
v2.7’s trail is crisp and sweet. 🥕🐇

Pre-merge checks (2 passed, 1 inconclusive)

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description Check	❓ Inconclusive	The pull request description is merely the unfilled template with no details about the implemented change, its motivation, or its behavior, making it too generic to understand the actual modifications from the description alone.	Please fill out the PR description with a concise summary of the change, its background, the current and new behavior, any tests or documentation added, and whether it introduces a breaking change.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the primary functional change — enforcing that only string-type IDs are registered to metaHashMap — and uses an imperative style without extraneous details or ambiguity.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

packages/register/src/common.ts (1)

88-101: Add tests to lock the contract; consider clarifying scope in code comments.

• Tests: cover objects where id is number, boolean, null/undefined, symbol, empty string, and valid string to ensure only valid strings are registered and registryApiAndOptionsMap is skipped otherwise.
• Comment: add a short note that genDefaultHashMap intentionally recurses only for the default registry (mergeRegistry’s first arg), while user registries are first-level-only (handled in the loop below), to avoid confusion during maintenance.

I can draft Jest/Vitest cases that assert metaHashMap.size and apisMap/optionsMap mutations for the above inputs.

docs/changelog/v2.7-upgrade-guide.md (3)

366-371: Clarify “first-level only” applies to user registries, not the built-in default registry.

To prevent misinterpretation, explicitly scope this rule to “用户注册表”; the engine still recursively flattens the built-in default registry during initialization.

Apply this edit:

-自 v2.7 起，我们只读取注册表对象的“第一层”。只有第一层的键会被注册进 `metaHashMap`。
+自 v2.7 起（仅针对“用户注册表”），我们只读取注册表对象的“第一层”。只有第一层的键会被注册进 `metaHashMap`。
+说明：内置的默认注册表仍会在引擎初始化时被递归展开并写入 `metaHashMap`，该规则不影响默认注册表的构建流程。

---

377-391: Avoid duplicate computed keys in one object example.

Using [META_APP.Materials] twice in a single object literal will have the latter override the former, which can distract readers from the main point. Split into two separate examples or annotate that they are independent cases.

-export default {
-  // 1) 在子层直接再次声明插件/配置项（无效，不会注册到 metaHashMap）
-  [META_APP.Materials]: {
-    'engine.plugins.test': testMeta
-  },
-
-  // 2) 通过 metas 在子层注入（无效，不会注册到 metaHashMap）
-  [META_APP.Materials]: {
-    options: {
-      displayComponentIds: ['engine.plugins.test'],
-      metas: [testMeta]
-    }
-  }
-}
+// 示例 A：在子层再次声明（无效）
+export const invalidA = {
+  [META_APP.Materials]: {
+    'engine.plugins.test': testMeta
+  }
+}
+// 示例 B：通过 metas 注入（无效）
+export const invalidB = {
+  [META_APP.Materials]: {
+    options: {
+      displayComponentIds: ['engine.plugins.test'],
+      metas: [testMeta]
+    }
+  }
+}

---

440-451: Reinforce the “first-level + reference by ID” pattern with one sentence.

Add a one-liner reminding that Materials 里仅通过 ID 引用，实际注册必须在第一层完成，避免读者误以为 displayComponentIds 会隐式注册。

   [META_APP.Materials]: {
     options: {
       displayComponentIds: ['engine.plugins.myMaterial']
     }
   }
 }
+// 说明：Materials 内仅通过 ID 引用；真正的“注册”必须在用户注册表的第一层完成。

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce07672 and 2575152.

📒 Files selected for processing (2)

• docs/changelog/v2.7-upgrade-guide.md (1 hunks)
• packages/register/src/common.ts (1 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: push-check

🔇 Additional comments (1)

packages/register/src/common.ts (1)

93-96: Stricter ID gate is correct and aligned with intent.

Guarding registration with typeof id === 'string' && id prevents non-string and empty IDs from entering metaHashMap. No functional regressions apparent.