Skip to content

Conversation

@edwinhuish
Copy link
Collaborator

@edwinhuish edwinhuish commented Sep 16, 2025

Description 描述

related #238

Summary by CodeRabbit

  • Bug Fixes

    • Prevents crashes when reading page metadata by handling read and JSON serialization failures more safely.
    • Invalid or malformed custom blocks no longer break processing; defaults are used when parsing fails.
    • More reliable change detection by computing and assigning metadata and raw content in a safer order.
  • Refactor

    • Simplified component parsing to surface parse errors directly, improving visibility and diagnostics during builds.

@coderabbitai
Copy link

coderabbitai bot commented Sep 16, 2025

Walkthrough

Revises page metadata reading and SFC parsing logic in packages/core/src/page.ts. Adds guarded error handling for read failures and JSON stringify, adjusts assignment order for meta/raw, removes catch-and-wrap around SFC parsing to let errors propagate, and safely ignores invalid custom block content during meta extraction.

Changes

Cohort / File(s) Summary of Changes
Core page parsing and meta handling
packages/core/src/page.ts
- Page.read(): add try/catch around meta read; guard JSON.stringify; compute changed after raw; assign meta/raw after computation; log errors via debug.error and early-return on read failure.
- parseSFC(): remove outer error-wrapping; let parser errors propagate; keep fallback parse call.
- tryPageMetaFromCustomBlock(): wrap cjAssign of block.content in try/catch; ignore parse errors, retain defaults.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant Page
  participant FS as File System
  participant Parser as SFC Parser
  participant Log as debug.error

  Caller->>Page: read()
  Page->>FS: readPageMetaFromFile()
  alt read success
    Page->>Page: JSON.stringify(meta)
    alt stringify success
      Page->>Page: raw computed
    else stringify fails
      Note over Page: raw = '' (unchanged/empty)
    end
    Page->>Page: set this.meta, this.raw
    Page->>Page: this.changed = (oldRaw != raw)
  else read fails
    Page->>Log: log error
    Page-->>Caller: return early
  end

  Note over Parser,Page: parseSFC() now lets errors propagate
  Note over Page: tryPageMetaFromCustomBlock() swallows invalid content errors
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble through pages with whiskered care,
Catching slips mid-hop, with a debonair flair.
If parsing storms rumble, I let thunder roll—
yet soften the thorns around the meta’s soul.
Tap-tap my paws: changed or not? I decide—
then thump a log, and bounce with pride. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix: #238 hot reload error" succinctly states the primary intent of the PR (a fix for the hot-reload error referenced by issue #238) and aligns with the changeset that adjusts error handling around page read and SFC parsing to address hot-reload failures; it is short, specific, and understandable for teammates scanning history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 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 8d753c4 and a37fab3.

📒 Files selected for processing (1)
  • packages/core/src/page.ts (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/core/src/page.ts (2)
packages/core/src/types.ts (1)
  • UserPageMeta (160-171)
packages/core/src/utils.ts (1)
  • debug (23-33)
🔇 Additional comments (3)
packages/core/src/page.ts (3)

68-88: Improved error handling and ordering in page metadata reading.

The changes implement more robust error handling for file reading and JSON serialization, with proper ordering of variable assignments. The early return on read failure prevents downstream errors, and the separate try-catch for JSON.stringify ensures the process can continue even if serialization fails.


109-120: Simplified SFC parsing with direct error propagation.

The removal of error wrapping allows parse errors to bubble up naturally to the caller, which is appropriate since the calling code (lines 102-104) already handles and contextualizes these errors properly. This change aligns with the PR objective of fixing hot reload errors by allowing more granular error handling.


162-168: Keep the defensive try/catch — validate/parse the source before calling cjAssign
cjAssign expects an object; malformed or non-object routeBlock.content can throw. Either parse/validate routeBlock.content into an object before calling cjAssign, or keep the try/catch to avoid breaking page metadata extraction.
Location: packages/core/src/page.ts:162-168.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.


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.

@FliPPeDround
Copy link
Member

这么多问题吗😀[扣脑袋]

@FliPPeDround FliPPeDround merged commit 9be53c4 into uni-helper:main Sep 16, 2025
3 of 4 checks passed
@edwinhuish edwinhuish deleted the fix_hot_reload branch September 16, 2025 03:01
@edwinhuish
Copy link
Collaborator Author

这么多问题吗😀[扣脑袋]

程序员存在的意义不就是写bug吗?0BA36C8B

@edwinhuish
Copy link
Collaborator Author

不过这次这个bug是之前的,parseSFC时,遇到无法解析的代码,就会出错。

@FliPPeDround
Copy link
Member

厉害呀,辛苦啦

@FliPPeDround
Copy link
Member

敬礼🫡

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