Tighten runtime packaging and version cache behavior - #554
Conversation
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 40 minutes and 25 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements caching for package version resolution with file-system-based invalidation, and introduces a publish-time backup/restore script that excludes source maps from npm pack tarballs while preserving them in local builds. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05fd5cceed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/wp-typia/tests/cli-package.test.ts (1)
473-479:⚠️ Potential issue | 🟡 MinorMake the pack-restore test independent of ambient skip flags.
The new local map assertions depend on
postpackrestore running, but this spawnednpm packinheritsWP_TYPIA_SKIP_POSTPACK_RESTORE=1if a developer or CI job has it set. Clear it for this command so the test always validates the default restore behavior.Proposed fix
const packResult = runCapturedCommand( 'npm', ['pack', '--json', '--pack-destination', packageRoot], { cwd: packageRoot, + env: { + ...process.env, + WP_TYPIA_SKIP_POSTPACK_RESTORE: '', + }, }, );Also applies to: 535-542
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/wp-typia/tests/cli-package.test.ts` around lines 473 - 479, The test's spawned npm pack inherits the WP_TYPIA_SKIP_POSTPACK_RESTORE env var and can skip the postpack restore; update the two runCapturedCommand invocations (the one that produces packResult and the similar call around lines 535-542) to explicitly clear WP_TYPIA_SKIP_POSTPACK_RESTORE in the env passed to runCapturedCommand (i.e., construct the command's env from process.env but delete or unset WP_TYPIA_SKIP_POSTPACK_RESTORE) so the pack process always runs the default postpack restore during the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/wp-typia-project-tools/src/runtime/package-versions.ts`:
- Around line 115-117: Add an English JSDoc block for the public function
invalidatePackageVersionsCache describing what it does (clears the internal
cachedPackageVersions), when callers should invoke it (e.g., in long‑lived
processes after package manifests or versions have been regenerated or updated),
and any side effects/behavior (synchronous, no return value). Place the JSDoc
directly above the exported function declaration so it documents the public
runtime API.
- Around line 67-83: The cache key produced by resolvePackageManifestLocation
can remain unchanged for rapid rewrites on filesystems with coarse mtimeMs,
causing stale getPackageVersions() results; modify
resolvePackageManifestLocation to include additional fast-changing identifiers
(e.g., stats.ino and stats.ctimeMs) in the cacheKey and, if those fields are not
available or reliable, compute and include a cheap short content fingerprint
(e.g., a truncated hash of fs.readFileSync(packageJsonPath)) as a fallback;
update references to resolvePackageManifestLocation, getPackageVersions, and
invalidatePackageVersionsCache as needed to ensure the cacheKey change is used
for lookups so rapid same-size edits are detected.
In `@packages/wp-typia/scripts/publish-runtime-maps.mjs`:
- Around line 39-45: The build currently treats a missing runtime directory as
fine because collectSourceMapRelativePaths(distRoot) returns [], allowing
preparePublishedRuntimeMaps to succeed; change preparePublishedRuntimeMaps to
explicitly check for the existence of distRoot (the runtime output directory
returned by getPublishRuntimeMapPaths) before proceeding and throw a clear Error
if the directory is missing so prepack fails fast; locate
preparePublishedRuntimeMaps and add a guard using fs.existsSync(distRoot) (or
equivalent) that throws a descriptive error referencing the missing distRoot
when the directory does not exist.
- Around line 77-85: The loop currently skips missing backups silently: in the
for (const relativePath of relativePaths) loop check backupPath existence and if
missing, instead of unconditionally continue, compute destinationPath and if
destinationPath already exists keep idempotency (continue) but if
destinationPath does NOT exist treat this as a genuine missing restore and throw
an error or call process.exit(1) so the script fails non‑zero; update the block
that uses backupPath, destinationPath, fs.existsSync, fs.mkdirSync and
fs.renameSync accordingly and ensure the overall script does not proceed to
delete backupRoot when that error occurs.
- Line 5: The constant DEFAULT_PACKAGE_ROOT currently uses import.meta.dirname
which isn't available on Node 20; replace it by deriving the file path from
import.meta.url: import fileURLToPath from 'url' and compute
DEFAULT_PACKAGE_ROOT with
path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") (i.e., remove
import.meta.dirname usage, add the fileURLToPath import, and call path.dirname
on the converted file path).
---
Outside diff comments:
In `@packages/wp-typia/tests/cli-package.test.ts`:
- Around line 473-479: The test's spawned npm pack inherits the
WP_TYPIA_SKIP_POSTPACK_RESTORE env var and can skip the postpack restore; update
the two runCapturedCommand invocations (the one that produces packResult and the
similar call around lines 535-542) to explicitly clear
WP_TYPIA_SKIP_POSTPACK_RESTORE in the env passed to runCapturedCommand (i.e.,
construct the command's env from process.env but delete or unset
WP_TYPIA_SKIP_POSTPACK_RESTORE) so the pack process always runs the default
postpack restore during the test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a0d87d1-27a5-4aaa-9a5a-9228afcc7410
📒 Files selected for processing (6)
packages/wp-typia-project-tools/src/runtime/package-versions.tspackages/wp-typia/package.jsonpackages/wp-typia/scripts/publish-runtime-maps.mjspackages/wp-typia/tests/bunli-prep.test.tspackages/wp-typia/tests/cli-package.test.tstests/unit/package-versions.test.ts
1 participant
Summary
dist-bunlisource maps from the publishedwp-typiatarball while restoring them locally after packpackage-versionsso long-lived processes recompute when package manifests changeTesting
Closes #550
Summary by CodeRabbit
Release Notes