perf: 移除子进程重启的代码，将文件操作改为异步，并加锁 #246
Conversation
WalkthroughIntroduces async and locked I/O for pages.json handling, converts synchronous utilities to async, makes the Vite plugin factory async, and lazily infers formatting metadata (indent/newline). Adds proper-lockfile dependency. Updates read paths to fs.promises. Adjusts validation flow to await checks without restart logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Vite as Vite (build/server)
participant Plugin as VitePluginUniPages (async)
participant Files as files.ts
participant Ctx as PageContext
participant FS as FS
participant Lock as proper-lockfile
Dev->>Vite: start
Vite->>Plugin: initialize (await)
Plugin->>Files: checkPagesJsonFile(path)
Files->>FS: stat/read/write as needed (async)
Files-->>Plugin: Promise<boolean>
Plugin-->>Vite: Promise<Plugin> resolved
Note over Ctx,FS: On pages update
Ctx->>Lock: lock(pages.json)
alt locked
Ctx->>FS: read pages.json (lazy format inference)
Ctx->>FS: write pages.json (async)
Ctx->>Lock: unlock
else lock failed
Ctx-->>Ctx: retry/propagate error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
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. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/core/src/page.ts (1)
32-43: Guard against undefinedthis.metawhen read fails to prevent runtime crash.
read()swallows errors and returns early, leavingthis.metaundefined. The non-null assertion will crash at runtime.Apply this diff:
public async getPageMeta(forceUpdate = false): Promise<PageMetaDatum> { if (forceUpdate || !this.meta) { await this.read() } - const { path, tabBar: _, ...others } = this.meta! + if (!this.meta) { + // Fallback to minimal defaults when meta can't be read + return { type: 'page', path: this.uri } + } + const { path, tabBar: _, ...others } = this.metapackages/core/src/index.ts (1)
31-41: Avoid making the plugin factory async; this is a breaking change for Vite users.Most Vite configs use
plugins: [uniPages()]synchronously. ReturningPromise<Plugin>forces consumers to refactor to an async config. Do async work inconfigResolvedinstead and keep the factory sync.Apply this diff:
-export async function VitePluginUniPages(userOptions: UserOptions = {}): Promise<Plugin> { +export function VitePluginUniPages(userOptions: UserOptions = {}): Plugin { let ctx: PageContext - // TODO: check if the pages.json file is valid - const resolvedPagesJSONPath = path.join( - process.env.VITE_ROOT_DIR || process.cwd(), - userOptions.outDir ?? 'src', - OUTPUT_NAME, - ) - await checkPagesJsonFile(resolvedPagesJSONPath) + // pages.json validity check is handled during configResolved/update flow
🧹 Nitpick comments (9)
packages/core/package.json (1)
62-62: Move @types/proper-lockfile to devDependencies (types are not needed at runtime).Keep runtime deps lean; ship only
proper-lockfilein dependencies and place the@typespackage under devDependencies.Apply this diff:
"dependencies": { "@babel/generator": "^7.28.3", "@babel/types": "^7.28.2", - "@types/proper-lockfile": "^4.1.4", "@uni-helper/uni-env": "^0.1.4", "@vue/compiler-sfc": "3.4.21", "ast-kit": "1.4.3", @@ "yaml": "^2.5.0" }, "devDependencies": { + "@types/proper-lockfile": "^4.1.4", "@antfu/utils": "^0.7.10", "@types/babel__generator": "^7.27.0", "@types/debug": "^4.1.12", "@types/lodash.groupby": "^4.6.9", "@types/node": "^20.15.0", "cross-env": "^10.0.0", "vite": "^5.4.1" }Also applies to: 81-89
packages/core/src/page.ts (2)
5-5: Use fs/promises (or namedpromises as fs) import for clarity and interop.Avoid default-importing
node:fsand then using.promises. This reduces ESM/CJS interop pitfalls and reads cleaner.Apply this diff:
-import fs from 'node:fs' +import { promises as fs } from 'node:fs'And update the read call:
- const content = await fs.promises.readFile(this.path.absolutePath, { encoding: 'utf-8' }) + const content = await fs.readFile(this.path.absolutePath, { encoding: 'utf-8' })Also applies to: 92-92
45-61: Ditto forgetTabBar: avoid destructuring whenthis.metais unset.Prevent undefined access if
read()failed.public async getTabBar(forceUpdate = false): Promise<TabBarItem & { index: number } | undefined> { if (forceUpdate || !this.meta) { await this.read() } - const { tabBar } = this.meta! + if (!this.meta) + return undefined + const { tabBar } = this.metapackages/core/src/index.ts (1)
45-61: Run the file check duringconfigResolved(or rely onctx.updatePagesJSON()which already checks).
updatePagesJSON()already callscheckPagesJsonFile. If you prefer an explicit check, do it here afterctxis constructed.Option A (minimal; rely on existing check inside updatePagesJSON): No change needed.
Option B (explicit check):
async configResolved(config) { ctx = new PageContext(userOptions, config.root) @@ ctx.setLogger(logger) - await ctx.updatePagesJSON() + await checkPagesJsonFile(ctx.resolvedPagesJSONPath) + await ctx.updatePagesJSON()packages/core/src/files.ts (3)
25-29: Fix JSDoc return type to match the signature (Promise)./** * 检查指定路径的pages.json文件，如果文件不存在或不是有效文件则创建一个空的pages.json文件 * @param path - 要检查的文件路径 - * @returns Promise<void> - 无返回值的异步函数 + * @returns Promise<boolean> - 处理成功返回 true，否则 false */
31-37: Ensure parent directory exists and handle directories when removing invalid targets.
- Create parent dirs before writing.
- Use
fs.rm(..., { recursive: true, force: true })to handle the rare case where a directory exists at the target path.Apply this diff:
-export async function checkPagesJsonFile(path: fs.PathLike): Promise<boolean> { - const createEmptyFile = (path: fs.PathLike) => { - return fs.promises.writeFile(path, JSON.stringify({ pages: [{ path: '' }] }, null, 2), { encoding: 'utf-8' }).then(() => true).catch(() => false) - } +export async function checkPagesJsonFile(path: fs.PathLike): Promise<boolean> { + const createEmptyFile = async (path: fs.PathLike) => { + try { + await fs.promises.mkdir(nodePath.dirname(String(path)), { recursive: true }) + await fs.promises.writeFile(path, JSON.stringify({ pages: [{ path: '' }] }, null, 2), { encoding: 'utf-8' }) + return true + } + catch { + return false + } + } - const unlink = (path: fs.PathLike) => { - return fs.promises.unlink(path).then(() => true).catch(() => false) - } + const unlink = (path: fs.PathLike) => { + return fs.promises.rm(path, { recursive: true, force: true }).then(() => true).catch(() => false) + }Add the missing import (outside the changed hunk):
import nodePath from 'node:path'
40-66: Optional: check the boolean result at call sites to avoid proceeding on invalid paths.When this function returns false, subsequent writes will fail (e.g., EISDIR). Either throw here or have callers branch on the result and log a clear error.
packages/core/src/context.ts (2)
388-395: Typo: renamerelasetoreleasefor clarity.No behavior change; just readability.
- const relase = await lockfile.lock(this.resolvedPagesJSONPath, { realpath: false }) + const release = await lockfile.lock(this.resolvedPagesJSONPath, { realpath: false }) @@ - await relase() // 释放文件锁 + await release() // 释放文件锁
425-454: Minor: avoid re-reading file info in concurrent calls.The lazy getters work, but concurrent callers could trigger
readInfoFromPagesJSON()multiple times. A simple in-flight promise cache would avoid duplicate reads.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
packages/core/package.json(2 hunks)packages/core/src/context.ts(6 hunks)packages/core/src/files.ts(1 hunks)packages/core/src/index.ts(2 hunks)packages/core/src/page.ts(2 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.tspackages/core/src/page.ts
🧬 Code graph analysis (2)
packages/core/src/index.ts (2)
packages/core/src/types.ts (1)
UserOptions(97-97)packages/core/src/files.ts (1)
checkPagesJsonFile(30-67)
packages/core/src/context.ts (1)
packages/core/src/files.ts (1)
checkPagesJsonFile(30-67)
⏰ 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). (2)
- GitHub Check: test (macos-latest, 20)
- GitHub Check: test (windows-latest, 20)
🔇 Additional comments (1)
packages/core/src/context.ts (1)
338-381: LGTM: lock-protected write with formatting preservation.Good use of
proper-lockfileand lazy indent/newline detection; lock acquisition is scoped to the actual write and only taken when content changes.Consider adding a small debug log around lock acquire/release to aid diagnosing stalls under contention.
1 participant
Description 描述
解决 uni pages 占用大量内存的bug
Summary by CodeRabbit
Bug Fixes
Refactor
Chores