fix: resolve non-portable types and enable verbatimModuleSyntax#180
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Too much diff to scan? Review this PR in Change Stack to start with the highest-impact changes. Warning Review limit reached
More reviews will be available in 18 minutes. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
📝 WalkthroughWalkthroughThis PR removes explicit ChangesType System Cleanup and App Auth Module Updates
Express and React Router Integration Type Contracts
Build Configuration and TypeScript Compiler Options
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
packages/express/src/@types/index.ts (1)
1-3: ⚡ Quick winUse type-only imports in this type-only declaration module.
On Line [1]–Line [3], these imports are only used in type positions. With
verbatimModuleSyntax, keeping them as value imports can preserve unnecessary runtime imports.Suggested diff
-import { LocalsWithSession } from "`@/lib/with-auth.ts`" -import { AuthInstance } from "`@aura-stack/auth`" -import { FromShapeToObject, Identities } from "`@aura-stack/auth/identity`" +import type { LocalsWithSession } from "`@/lib/with-auth.ts`" +import type { AuthInstance } from "`@aura-stack/auth`" +import type { FromShapeToObject, Identities } from "`@aura-stack/auth/identity`"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/express/src/`@types/index.ts around lines 1 - 3, The current imports (LocalsWithSession, AuthInstance, FromShapeToObject, Identities) in this type-only declaration module are imported as value imports; change them to type-only imports by using `import type` for the symbols LocalsWithSession, AuthInstance, FromShapeToObject, and Identities so the compiler/transformer won’t emit unnecessary runtime imports—update the import statements that reference "`@/lib/with-auth.ts`" and "`@aura-stack/auth`" / "`@aura-stack/auth/identity`" accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/device/package.json`:
- Around line 18-19: The "clean:cts" npm script fails when the dist directory is
missing which breaks "prepublishOnly"; update the "clean:cts" script (referenced
by the package.json script keys "clean:cts" and "prepublishOnly") to be no-op
when dist is absent — e.g., guard the find command with a directory existence
check or append safe-fail handling so the command exits successfully if dist
doesn't exist before running build.
In `@packages/elysia/package.json`:
- Line 19: The package.json script "clean:cts" currently fails when dist/ is
absent; update the "clean:cts" script so it becomes no-op instead of exiting
non-zero when dist is missing — for example, make it check that dist exists
before running find, or use a find invocation that tolerates no matches (or
append a fallback `|| true`), or switch to an rm -f style pattern that safely
ignores missing files; modify the "clean:cts" script entry accordingly.
In `@packages/express/package.json`:
- Line 19: The npm script "clean:cts" in package.json currently runs find on
dist and will fail if dist is missing; update the "clean:cts" script so it first
checks for the existence of the dist directory (or otherwise swallows a
missing-directory error) before invoking find — e.g., make the script
conditional on [ -d dist ] && find ... or append a no-fail fallback (|| true) so
the cleanup is a no-op when dist/ is absent; update the "clean:cts" entry
accordingly in package.json.
In `@packages/hono/package.json`:
- Line 19: The "clean:cts" npm script currently runs find on dist and fails when
dist is missing; update the "clean:cts" entry in package.json so it first checks
for the existence of the dist directory (e.g., using a directory-existence test
like test -d or [ -d ]) and only runs the find ... -delete when dist exists,
making the script a no-op and idempotent when dist/ is absent.
In `@packages/integration/package.json`:
- Around line 19-20: The prepublishOnly script currently invokes the clean:cts
script unguarded which fails when dist doesn't exist; update the package.json so
clean:cts is safe to run when dist is missing (e.g., make clean:cts check for
the existence of the dist directory before running the find/delete command) or
reorder prepublishOnly to run the build before calling clean:cts; adjust the
"clean:cts" and/or "prepublishOnly" script entries accordingly so pnpm publish
does not exit non‑zero when dist is absent.
In `@packages/rate-limiter/package.json`:
- Line 19: The "clean:cts" npm script fails when the dist directory is missing;
update the package.json scripts entry "clean:cts" so it no-ops harmlessly when
dist doesn't exist (e.g., guard the find invocation with a directory-exists
check or suppress the missing-directory error and return success) so running
pnpm clean:cts on a fresh clone does not fail.
In `@packages/react-router/package.json`:
- Line 19: The "clean:cts" npm script in package.json can fail when the dist
directory is missing; update the "clean:cts" script to avoid a non-zero exit by
either (A) guarding the find command with a directory existence check (only run
find if dist exists) or (B) making the command tolerant of no-matches by
swallowing a non-zero exit (e.g., append a no-op success). Modify the script
entry named "clean:cts" accordingly so CI/scripts do not fail when dist/ is
absent.
In `@packages/react-router/src/`@types/core.ts:
- Line 1: The file currently does a runtime re-export with `export * from
"`@aura-stack/react/types`"` which will try to emit JS for a types-only module;
change it to a type-only re-export so no runtime import is emitted (e.g. replace
the export with a type-only re-export such as using TypeScript's type-only
export form for the module) and ensure the module path in
`packages/react-router/src/@types/core.ts` refers to the types entry
(`"`@aura-stack/react/types`"`) as a type-only export to avoid runtime resolution
errors.
In `@packages/react/package.json`:
- Line 19: The "clean:cts" npm script currently runs find against "dist" without
checking if the directory exists, causing failures when "dist" is absent; update
the "clean:cts" script entry in package.json so it first checks that the "dist"
directory exists and only then runs the find delete command (i.e., add a guard
that tests for the existence of "dist" before invoking find) to restore the
previous resilient behavior.
---
Nitpick comments:
In `@packages/express/src/`@types/index.ts:
- Around line 1-3: The current imports (LocalsWithSession, AuthInstance,
FromShapeToObject, Identities) in this type-only declaration module are imported
as value imports; change them to type-only imports by using `import type` for
the symbols LocalsWithSession, AuthInstance, FromShapeToObject, and Identities
so the compiler/transformer won’t emit unnecessary runtime imports—update the
import statements that reference "`@/lib/with-auth.ts`" and "`@aura-stack/auth`" /
"`@aura-stack/auth/identity`" accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27686bc8-8f94-46a6-82d6-62407da014e9
📒 Files selected for processing (47)
apps/bun/src/lib/auth.tsapps/cloudflare/src/auth.tsapps/deno/src/auth.tsapps/oak/src/auth.tsapps/supabase/functions/_shared/auth.tspackages/core/src/actions/signIn/authorization-url.tspackages/core/src/actions/signIn/authorization.tspackages/core/src/router/context.tspackages/core/src/shared/assert.tspackages/core/src/shared/crypto.tspackages/core/src/shared/logger.tspackages/core/src/validator/registry.tspackages/core/test/actions/callback/userinfo.test.tspackages/core/test/identity.test.tspackages/core/test/oauth.test.tspackages/core/test/types.test-d.tspackages/core/tsconfig.jsonpackages/device/package.jsonpackages/elysia/package.jsonpackages/express/package.jsonpackages/express/src/@types/index.tspackages/express/src/createAuth.tspackages/hono/package.jsonpackages/integration/package.jsonpackages/jose/src/deriveKey.tspackages/jose/src/encrypt.tspackages/jose/tsconfig.jsonpackages/next/package.jsonpackages/rate-limiter/package.jsonpackages/react-router/package.jsonpackages/react-router/src/@types/api.tspackages/react-router/src/@types/core.tspackages/react-router/src/@types/index.tspackages/react-router/src/client.tspackages/react-router/src/client.tsxpackages/react-router/src/createAuth.tspackages/react-router/src/index.tspackages/react-router/src/lib/api.tspackages/react-router/test/context/provider.test.tsxpackages/react-router/test/context/redirect.test.tsxpackages/react-router/tsconfig.jsonpackages/react-router/tsdown.config.tspackages/react/package.jsonpackages/react/src/context.tsxpackages/react/src/hooks.tspackages/react/tsconfig.jsonpackages/react/tsdown.config.ts
💤 Files with no reviewable changes (1)
- packages/react-router/src/client.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request fixes non-portable types across the auth packages, including
@aura-stack/react-routerand@aura-stack/express.These packages were causing TypeScript errors when consuming the
apiobject returned bycreateAuth, as some exported types could not be properly named or referenced in generated declaration files.Additionally, this PR enables the TypeScript
verbatimModuleSyntaxcompiler option. This enforces explicit type-only imports and exports by requiring thetypekeyword when importing or exporting types, resulting in clearer module boundaries and improved type portability.