Skip to content

Conversation

KeJunMao
Copy link
Member

@KeJunMao KeJunMao commented Sep 24, 2025

Description 描述

此 PR 为 H5 实现了初始化 pages.json 的能力,启动时,即使目录下没有 pages.json 也不会崩溃。

当前 uni-app 在 h5 环境是直接用的 vite server,其 initEasycom 函数是后置的执行的,因此可以通过拦截 readFileSync 来生成并返回一个空的 pages.json 来骗过 uni cli 的初始化。

日后 uni 更新到 createBuilder 后,也可以用这种方式

https://github.com/dcloudio/uni-app/blob/c4d75050a2ab22c6ad2019df6bd1a6c8f15776d3/packages/vite-plugin-uni/src/cli/action.ts#L47-L50

Linked Issues 关联的 Issues

Additional context 额外上下文

Summary by CodeRabbit

  • New Features

    • Automatically initializes the pages configuration with sensible defaults when missing, enabling smoother first-time and recovery scenarios.
    • Ensures reads of the pages configuration are non-blocking, allowing the app to continue operating while the file is prepared.
  • Bug Fixes

    • Prevents startup errors when the pages configuration file is absent or temporarily unreadable.
    • Limits setup to relevant environments, avoiding unnecessary initialization where not needed.

Copy link

coderabbitai bot commented Sep 24, 2025

Walkthrough

Introduces EMPTY_PAGES_JSON_CONTENTS, refactors file creation to use it, adds setupPagesJsonFile to proxy fs.readFileSync for pages.json, updates checkPagesJsonFile to accept customizable contents, and changes plugin initialization to conditionally set up pages.json only in H5 environments.

Changes

Cohort / File(s) Summary
Constants: default pages.json contents
packages/core/src/constant.ts
Added exported constant EMPTY_PAGES_JSON_CONTENTS with value { "pages": [{ "path": "" }] }.
File utilities: creation and read proxy
packages/core/src/files.ts
Updated checkPagesJsonFile(path, contents = EMPTY_PAGES_JSON_CONTENTS); createEmptyFile now writes provided contents; added setupPagesJsonFile(path) that temporarily proxies fs.readFileSync to handle reads for the target path, ensuring fallback to EMPTY_PAGES_JSON_CONTENTS and on-demand creation. Imported normalizePath and EMPTY_PAGES_JSON_CONTENTS.
Plugin initialization guard (H5 only)
packages/core/src/index.ts
Replaced unconditional checkPagesJsonFile with conditional setupPagesJsonFile(resolvedPagesJSONPath) when isH5 is true; updated imports accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Plugin as VitePluginUniPages
  participant Env as isH5
  participant Files as setupPagesJsonFile
  participant FS as fs.readFileSync
  participant Disk as File System

  Plugin->>Env: check H5 environment
  alt H5 == true
    Plugin->>Files: setupPagesJsonFile(pages.json path)
    Note right of Files: Install temporary readFileSync proxy
  else
    Note over Plugin: Skip pages.json setup
  end
Loading
sequenceDiagram
  autonumber
  participant App as Caller
  participant FS as fs.readFileSync (proxied)
  participant Util as checkPagesJsonFile
  participant Disk as File System

  App->>FS: readFileSync(target path)
  alt First read after proxy install
    FS->>Disk: Try real read
    alt Read succeeds
      FS-->>App: Return real data
      Note over FS: Restore original readFileSync
    else Read fails
      FS->>Util: checkPagesJsonFile(target, contents=EMPTY_PAGES_JSON_CONTENTS)
      Util->>Disk: Ensure file exists (create if missing)
      Note over FS: Restore original readFileSync
      FS-->>App: Return EMPTY_PAGES_JSON_CONTENTS
    end
  else Subsequent reads
    FS->>Disk: Use restored readFileSync
    Disk-->>App: Return current file contents
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

size/XXL

Suggested reviewers

  • skiyee
  • nei1ee

Poem

A nibble of pages, a proxy in place,
I hop through reads with featherlight grace.
If files aren’t there, I won’t be distraught—
I conjure a stub, just as I ought.
H5 wakes; I twitch my ears—done!
Now back to burrows—shipping’s fun. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% 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
Title Check ✅ Passed The title "feat: 为 H5 实现初始化 pages.json 的能力" clearly and concisely summarizes the main change — adding the ability to initialize pages.json for H5 to avoid startup errors — and directly matches the PR objectives and code changes described. It is specific and focused, making the primary intent obvious to reviewers scanning history.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-init-pages-json-file-h5

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: 1

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

7-7: Prefer a schema‑safe empty pages.json

Using an empty path may violate uni-app expectations. Safer default is an empty pages array.

Apply this diff:

-export const EMPTY_PAGES_JSON_CONTENTS = '{ "pages": [{ "path": "" }] }'
+export const EMPTY_PAGES_JSON_CONTENTS = JSON.stringify({ pages: [] })

Please confirm uni-cli init accepts an empty pages array in H5.

packages/core/src/files.ts (3)

27-31: Fix JSDoc return type (now returns boolean)

Doc says Promise, but the function returns Promise.

Apply this diff:

- * @returns Promise<void> - 无返回值的异步函数
+ * @returns Promise<boolean> - 是否可用/已成功创建

31-34: Create parent directory before writing pages.json

Write will fail if the parent dir doesn’t exist. Ensure mkdir -p first.

Apply this diff:

-export async function checkPagesJsonFile(path: fs.PathLike, contents: string = EMPTY_PAGES_JSON_CONTENTS): Promise<boolean> {
-  const createEmptyFile = (path: fs.PathLike) => {
-    return fs.promises.writeFile(path, contents, { encoding: 'utf-8' }).then(() => true).catch(() => false)
-  }
+export async function checkPagesJsonFile(path: fs.PathLike, contents: string = EMPTY_PAGES_JSON_CONTENTS): Promise<boolean> {
+  const createEmptyFile = (path: fs.PathLike) => {
+    return fs.promises
+      .mkdir(nodePath.dirname(String(path)), { recursive: true })
+      .then(() => fs.promises.writeFile(path, contents, { encoding: 'utf-8' }))
+      .then(() => true)
+      .catch(() => false)
+  }

Additional import needed:

import nodePath from 'node:path'

5-6: Missing import for nodePath used in diffs above

Required for mkdir/resolve in the suggested changes.

Apply this diff:

-import { normalizePath } from 'vite'
+import nodePath from 'node:path'
+import { normalizePath } from 'vite'
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4dfdcc and 56f2a3e.

📒 Files selected for processing (3)
  • packages/core/src/constant.ts (1 hunks)
  • packages/core/src/files.ts (3 hunks)
  • packages/core/src/index.ts (3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-28T03:00:09.715Z
Learnt from: xiaohe0601
PR: uni-helper/vite-plugin-uni-pages#229
File: packages/core/client.d.ts:9-11
Timestamp: 2025-08-28T03:00:09.715Z
Learning: In uni-helper/vite-plugin-uni-pages, global type declarations like `definePage` are kept in a separate `client.d.ts` file and exposed via the `./client` export. Users need to manually add `uni-helper/vite-plugin-uni-pages/client` to their tsconfig.json to access these global declarations, which is documented as the intended usage pattern.

Applied to files:

  • packages/core/src/index.ts
🧬 Code graph analysis (2)
packages/core/src/files.ts (1)
packages/core/src/constant.ts (1)
  • EMPTY_PAGES_JSON_CONTENTS (7-7)
packages/core/src/index.ts (1)
packages/core/src/files.ts (1)
  • setupPagesJsonFile (70-90)
🔇 Additional comments (1)
packages/core/src/index.ts (1)

6-6: H5‑only setup is on point

Conditionally installing the pages.json bootstrap only for H5 aligns with the runtime behavior you described. Import changes are consistent.

Confirm that isH5 is true in the H5 dev server process where uni-cli attempts to read pages.json.

Also applies to: 19-19, 41-43

Comment on lines +70 to +90
export function setupPagesJsonFile(path: string) {
const _readFileSync = fs.readFileSync
fs.readFileSync = new Proxy(fs.readFileSync, {
apply(target, thisArg, argArray) {
if (typeof argArray[0] === 'string' && normalizePath(argArray[0]) === normalizePath(path)) {
try {
const data = _readFileSync.apply(thisArg, argArray as any)
fs.readFileSync = _readFileSync
return data
}
catch {
checkPagesJsonFile(path).then(() => {
fs.readFileSync = _readFileSync
})
}
return EMPTY_PAGES_JSON_CONTENTS
}
return Reflect.apply(target, thisArg, argArray)
},
})
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

🧩 Analysis chain

Make the fs.readFileSync proxy robust (relative paths, Buffer vs string, idempotency)

  • Match absolute and relative paths; normalize with process.cwd().
  • Return Buffer when caller expects Buffer.
  • Prevent double‑patching across multiple plugin instances.

Apply this diff:

-export function setupPagesJsonFile(path: string) {
-  const _readFileSync = fs.readFileSync
-  fs.readFileSync = new Proxy(fs.readFileSync, {
-    apply(target, thisArg, argArray) {
-      if (typeof argArray[0] === 'string' && normalizePath(argArray[0]) === normalizePath(path)) {
-        try {
-          const data = _readFileSync.apply(thisArg, argArray as any)
-          fs.readFileSync = _readFileSync
-          return data
-        }
-        catch {
-          checkPagesJsonFile(path).then(() => {
-            fs.readFileSync = _readFileSync
-          })
-        }
-        return EMPTY_PAGES_JSON_CONTENTS
-      }
-      return Reflect.apply(target, thisArg, argArray)
-    },
-  })
-}
+export function setupPagesJsonFile(path: string) {
+  // idempotency guard
+  if ((fs as any).__uniPagesRFSProxyInstalled)
+    return
+  const _readFileSync = fs.readFileSync
+  ;(fs as any).__uniPagesRFSProxyInstalled = true
+  fs.readFileSync = new Proxy(fs.readFileSync, {
+    apply(target, thisArg, argArray) {
+      const req0 = argArray[0]
+      if (typeof req0 === 'string') {
+        // normalize to absolute for comparison
+        const reqAbs = nodePath.isAbsolute(req0) ? req0 : nodePath.resolve(process.cwd(), req0)
+        if (normalizePath(reqAbs) === normalizePath(path)) {
+          try {
+            const data = _readFileSync.apply(thisArg, argArray as any)
+            fs.readFileSync = _readFileSync
+            ;(fs as any).__uniPagesRFSProxyInstalled = false
+            return data
+          }
+          catch {
+            // ensure file asynchronously, then unpatch
+            checkPagesJsonFile(path).then(() => {
+              fs.readFileSync = _readFileSync
+              ;(fs as any).__uniPagesRFSProxyInstalled = false
+            })
+          }
+          // respect caller’s expected return type
+          const opt = argArray[1]
+          const wantsString =
+            typeof opt === 'string' ||
+            (opt && typeof opt === 'object' && typeof opt.encoding === 'string' && opt.encoding !== null)
+          return wantsString ? EMPTY_PAGES_JSON_CONTENTS : Buffer.from(EMPTY_PAGES_JSON_CONTENTS)
+        }
+      }
+      return Reflect.apply(target, thisArg, argArray)
+    },
+  })
+}

And add this import near the top of the file:

import nodePath from 'node:path'

Run this to check for other call sites of checkPagesJsonFile potentially relying on its old return type or contents:


🏁 Script executed:

#!/bin/bash
rg -nP --type=ts -C2 '\bcheckPagesJsonFile\s*\('

Length of output: 1075


Make the fs.readFileSync proxy robust (idempotent, resolve relative paths, preserve Buffer/string)

Apply the suggested diff (adds an idempotency guard __uniPagesRFSProxyInstalled, uses node:path.resolve(process.cwd(), ...) for comparison, and returns Buffer when caller expects Buffer). Also add import nodePath from 'node:path'.

Verified: checkPagesJsonFile is declared at packages/core/src/files.ts:31 as Promise and is awaited at packages/core/src/context.ts:338; no other callers found.

🤖 Prompt for AI Agents
In packages/core/src/files.ts around lines 70 to 90, the fs.readFileSync proxy
needs to be made idempotent, resolve relative paths with node:path, and preserve
Buffer/string return types: add `import nodePath from 'node:path'` at top,
introduce a module-scoped guard `__uniPagesRFSProxyInstalled` and return early
if set so the proxy is installed only once, compare targets using
`nodePath.resolve(process.cwd(), path)` (resolve both argArray[0] and the
configured path) to handle relative paths, and when the original readFileSync
returns a Buffer/string forward that exact value to the caller (do not coerce to
string); ensure the fallback EMPTY_PAGES_JSON_CONTENTS is returned as a Buffer
if the caller expected a Buffer. Set the guard after installing the proxy.

@edwinhuish
Copy link
Collaborator

大佬,我好像在处理 pages.json 的条件编译时,修正了这个支持。

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