refactor: consolidate fileURLToPath path resolution into resolveEntryPath()#702
refactor: consolidate fileURLToPath path resolution into resolveEntryPath()#702james-elicx merged 1 commit intomainfrom
Conversation
…Path() Replace the repeated `fileURLToPath(new URL(rel, import.meta.url)).replace(/\\/g, "/")` idiom across entry generators with a single shared utility. Add `resolveEntryPath(rel, base)` to runtime-entry-module.ts, then use it in app-rsc-entry.ts (16 call sites), pages-server-entry.ts (8 call sites), and have resolveRuntimeEntryModule delegate to it. Replace `const __dirname = path.dirname(fileURLToPath(import.meta.url))` in index.ts and tpr.ts with `import.meta.dirname` (Node 22+, already used throughout the test suite).
commit: |
|
/bigbonk review |
|
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean refactor. Verified the following:
resolveEntryPath is a correct drop-in replacement. Every replaced call site used exactly fileURLToPath(new URL(rel, base)).replace(/\\/g, "/") — the helper encapsulates this precisely. No call sites had variations (different base, missing backslash replace, etc.).
resolveRuntimeEntryModule delegation is safe. The old code applied replace(/\\/g, "/") only on the return value, not on the filePath passed to fs.existsSync. The new code applies it before existsSync, but this is fine — fs.existsSync accepts forward slashes on Windows, and on POSIX there are no backslashes to replace.
import.meta.dirname replacements in index.ts and tpr.ts are correct. Both files are regular source modules (not generated entries), so import.meta.dirname (Node 22+, already used elsewhere in the codebase per the PR description) is a safe substitute for path.dirname(fileURLToPath(import.meta.url)). The tpr.ts change is slightly more subtle — old code used new URL(".", import.meta.url) to get the directory, but the result is the same.
pages-server-entry.ts sanitizeDestination import — the old code used path.resolve(__dirname, "../config/config-matchers.js").replace(/\\/g, "/") where __dirname was derived from fileURLToPath(import.meta.url). The new code uses resolveEntryPath("../config/config-matchers.js", import.meta.url) which is semantically identical.
No remaining hand-rolled call sites. Searched for fileURLToPath(new URL( in packages/vinext/src/ — the only real usage is the consolidated one in runtime-entry-module.ts. Remaining matches are in comments or generated code for unrelated purposes (og-inline/resvg WASM inlining).
No path.dirname(fileURLToPath(import.meta.url)) patterns left — verified no remaining instances.
Net -17 lines with a single source of truth for the idiom. LGTM.
|
Reviewed and approved PR #702. Here's a summary of what I verified:
|
1 participant
Summary
resolveEntryPath(rel, base)toentries/runtime-entry-module.tsas the single owner of thefileURLToPath(new URL(rel, base)).replace(/\\/g, "/")idiomapp-rsc-entry.tsand 8 inpages-server-entry.tswith the shared helper;resolveRuntimeEntryModulealso delegates to itconst __dirname = path.dirname(fileURLToPath(import.meta.url))inindex.tsandcloudflare/tpr.tswithimport.meta.dirname(Node 22+, already used throughout the test suite), removing now-unused imports