feat: add lunalink and enjoy route inference #561
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@yoannfleurydev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 52 seconds before requesting another review. ⌛ 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
WalkthroughThe pull request adds the dependency Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant LL as lunalink
participant RT as Routes Definition
UI->>LL: Invoke lunalink(route(), params)
LL->>RT: Retrieve URL from route definition (using const assertion)
RT-->>LL: Return constant URL
LL-->>UI: Return processed URL
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (12)
package.json(1 hunks)src/features/account/routes.ts(1 hunks)src/features/admin/routes.ts(1 hunks)src/features/app/routes.ts(1 hunks)src/features/management/routes.ts(1 hunks)src/features/repositories/AdminRepositoryActions.tsx(2 hunks)src/features/repositories/PageAdminRepositories.tsx(2 hunks)src/features/repositories/PageAdminRepository.tsx(2 hunks)src/features/repositories/routes.ts(1 hunks)src/features/users/AdminUserActions.tsx(2 hunks)src/features/users/PageAdminUsers.tsx(2 hunks)src/features/users/routes.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/features/management/routes.ts
🔇 Additional comments (22)
package.json (1)
45-45: Adding lunalink for type-safe route generation.The addition of
@bearstudio/lunalinkwill help enforce type safety in route generation across the application, ensuring parameters needed for routes are properly provided.src/features/users/PageAdminUsers.tsx (2)
3-3: Good addition of the lunalink import.This aligns with the PR objectives to enhance route inference capabilities.
131-133: Enhanced route typing with lunalink.The change from direct route function calls to the lunalink pattern improves type safety. Now the route definition and parameters are clearly separated, allowing better static analysis and preventing parameter-related errors.
src/features/repositories/PageAdminRepositories.tsx (2)
3-3: Consistent use of lunalink import.Good job maintaining consistency in the import pattern across different files.
109-111: Better type safety for repository route generation.Converting to the lunalink function follows the same pattern as in the users module, creating a consistent approach to route parameter handling throughout the application.
src/features/users/AdminUserActions.tsx (2)
3-3: Proper lunalink import.Consistent with other files in the PR.
104-104: Improved route parameter safety.The conversion to lunalink provides better type checking for route parameters. This ensures that required parameters (like user ID) aren't accidentally omitted, making the codebase more robust.
src/features/repositories/PageAdminRepository.tsx (2)
3-3: Added import for lunalink packageThe import of
lunalinkfrom@bearstudio/lunalinkaligns with the PR objective to add lunalink support for route handling.
69-71: URL generation using lunalinkThe code now uses
lunalinkto generate the URL instead of directly calling the route function with parameters. This provides better type safety and route management.src/features/users/routes.ts (2)
5-6: Improved type safety withas constassertionsAdding
as constto route function return values ensures they're treated as literal types rather than general strings, improving type safety throughout the application.
7-7:✅ Verification successful
Route parameter handling changed
The
usermethod signature has been changed from accepting a parameter object to using a parameterized route format (:id). This is consistent with the lunalink approach but constitutes a breaking change for any code that directly calls this method without usinglunalink.Please ensure all callsites that use this route method have been updated to use the new signature with lunalink.
🏁 Script executed:
#!/bin/bash # Search for all instances where ROUTES_USERS.admin.user is called directly without lunalink rg "ROUTES_USERS\.admin\.user\(" --type tsLength of output: 262
Route parameter update verified – all callsites now use lunalink
I reviewed the callsite usages, and both occurrences in
src/features/users/PageAdminUsers.tsxandsrc/features/users/AdminUserActions.tsxcallROUTES_USERS.admin.user()vialunalink, confirming that the new parameterized route (:id) is consistently applied. No further changes are required.src/features/repositories/AdminRepositoryActions.tsx (3)
3-3: Added import for lunalink packageThe import of
lunalinkfrom@bearstudio/lunalinkis consistent with the PR objective.
58-60: URL generation using lunalinkConverting direct route function calls to use the
lunalinkfunction provides better type safety and consistency with the rest of the application.
67-69: URL generation using lunalink for edit actionSimilar to the previous change, this properly implements the lunalink pattern for the edit action URL.
src/features/admin/routes.ts (1)
2-2: Enhanced type safety withas constassertionAdding
as constto the route function return value improves type safety by treating it as a literal type.src/features/app/routes.ts (2)
2-2: Enhanced type safety with const assertion.Adding
as constto the return value of therootfunction ensures that TypeScript treats it as a literal string type '/app' rather than a general string type. This provides more precise type information that can be used for route inference.
6-8: Improved type handling in conditional return.The changes to the
baseUrlfunction improve type safety by:
- Adding
as constto ensure the empty string is treated as a literal type- Including a
@ts-expect-errorcomment to suppress expected TypeScript errors from the conditional return typesThis maintains the guard against double slashes in URLs while enhancing type checking.
src/features/account/routes.ts (2)
6-6: Enhanced type safety with const assertion.Using
as constassertion for the app root path ensures that TypeScript treats it as a literal string type rather than a general string type, which enables more precise type checking and better route inference.
9-11: Enhanced type safety across admin route definitions.The addition of
as constassertions to all admin route paths (root, profile, and email) creates consistent literal type treatment throughout the routing system. This is a good practice that enables stronger type checking and better IDE support.src/features/repositories/routes.ts (3)
6-6: Enhanced type safety with const assertion.Using
as constfor the app root path ensures that TypeScript treats it as a literal string type, which enables more precise type checking for route definitions.
9-10: Enhanced type safety for root and create routes.Adding
as constassertions to the root and create route paths improves type safety and enables better route inference with the lunalink dependency mentioned in the PR description.
11-12: Improved parameterized route handling with const assertions.The repository and update route definitions now:
- Use a parameterized format with
:idplaceholder- Include
as constassertions for literal type treatmentThis approach provides better type safety while maintaining the dynamic nature of these routes when used with the lunalink library.
src/features/admin/routes.ts
Outdated
| baseUrl: () => | ||
| // @ts-expect-error see comment above | ||
| ROUTES_ADMIN.root === ('/' as const) ? '' : ROUTES_ADMIN.root, |
There was a problem hiding this comment.
Logic error in baseUrl implementation
There's a critical issue in this change. The code now compares the function reference (ROUTES_ADMIN.root) directly to a string literal ('/'), which will always fail. The original comparison likely called the function (ROUTES_ADMIN.root()) to compare its return value.
This change will break the baseUrl functionality. Apply this fix:
- baseUrl: () =>
- // @ts-expect-error see comment above
- ROUTES_ADMIN.root === ('/' as const) ? '' : ROUTES_ADMIN.root,
+ baseUrl: () =>
+ ROUTES_ADMIN.root() === ('/' as const) ? '' : ROUTES_ADMIN.root(),Alternatively, if you want to maintain the as const type safety:
- baseUrl: () =>
- // @ts-expect-error see comment above
- ROUTES_ADMIN.root === ('/' as const) ? '' : ROUTES_ADMIN.root,
+ baseUrl: () =>
+ (ROUTES_ADMIN.root() === '/' ? '' : ROUTES_ADMIN.root()) as const,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| baseUrl: () => | |
| // @ts-expect-error see comment above | |
| ROUTES_ADMIN.root === ('/' as const) ? '' : ROUTES_ADMIN.root, | |
| baseUrl: () => | |
| ROUTES_ADMIN.root() === ('/' as const) ? '' : ROUTES_ADMIN.root(), |
yoannfleurydev
force-pushed
the
feat/add-lunalink-and-enjoy-route-types
branch
from
d10fc4a to
7b10ba5
Compare
yoannfleurydev
force-pushed
the
feat/add-lunalink-and-enjoy-route-types
branch
from
7b10ba5 to
d795c18
Compare
|
| root: () => `${ROUTES_ADMIN.baseUrl()}/repositories`, | ||
| create: () => `${ROUTES_REPOSITORIES.admin.root()}/create`, | ||
| repository: (params: { id: string }) => | ||
| `${ROUTES_REPOSITORIES.admin.root()}/${params.id}`, | ||
| update: (params: { id: string }) => | ||
| `${ROUTES_REPOSITORIES.admin.root()}/${params.id}/update`, | ||
| root: () => `${ROUTES_ADMIN.baseUrl()}/repositories` as const, | ||
| create: () => `${ROUTES_REPOSITORIES.admin.root()}/create` as const, | ||
| repository: () => `${ROUTES_REPOSITORIES.admin.root()}/:id` as const, | ||
| update: () => `${ROUTES_REPOSITORIES.admin.root()}/:id/update` as const, |
There was a problem hiding this comment.
Why are we still defining routes as functions? I think that with Lunalink, we will no longer provide params, so this should simplify both the declaration and usage.
For example
repository: `${ROUTES_REPOSITORIES.admin.root}/:id` as const,
Summary by CodeRabbit
New Features
Refactor
Chores