-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(api): apply function style convention across api and web #67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sam821203
wants to merge
5
commits into
develop
Choose a base branch
from
refactor/function-style
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
545f0ec
refactor(api): apply function style convention across api and web
sam821203 e9e681d
feat(api): add cloudinary optimized image url for product responses
sam821203 0b5fe39
refactor(api): add explicit return types to repositories and route ha…
sam821203 9363965
chore(devops): simplify eslint overrides for api and ecommerce
sam821203 f61d12f
chore(devops): fix function-style rule glob pattern for nested paths
sam821203 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| --- | ||
| description: Function style convention — arrow functions for non-components, function declarations for React components; use when writing or reviewing TS/TSX | ||
| globs: "**/*.{ts,tsx}" | ||
| alwaysApply: false | ||
| --- | ||
|
|
||
| # Function Style Convention | ||
|
|
||
| Unify function style across the codebase: | ||
|
|
||
| ## Use arrow functions | ||
|
|
||
| For **all non-component code** (helpers, API functions, handlers, mappers, hooks, async functions, etc.): | ||
|
|
||
| - **Format**: `const name = (params): ReturnType => { ... }` or `export const name = (params): ReturnType => { ... }` | ||
| - **Async**: `export const name = async (params): Promise<ReturnType> => { ... }` | ||
| - **Single expression**: `const name = (x: T): R => expression;` when the body is a single return. | ||
|
|
||
| Examples: `mapHttpError`, `getApiUrl`, `request`, `buildProductsQuery`, `getProducts`, `toUserResponse`, `toListItem`, `healthHandler`, `useNotification`, `useLocalStorage`, `getErrorMessage`, `formatDateTime`, `notifyError`. | ||
|
|
||
| ## Use `function` declarations | ||
|
|
||
| For **React components** (anything that returns JSX or is used as `<Component />`): | ||
|
|
||
| - **Format**: `export function ComponentName(props): React.ReactElement` or `function ComponentName(props) { ... }` | ||
| - Keep subcomponents (e.g. `ProductImageUploadField`, `LayoutContent`, `GroupLinks`) and providers (e.g. `NotificationProvider`, `DataBoundary`) as `function` as well. | ||
|
|
||
| Examples: `ProductsPage`, `ProductDetailPage`, `DataBoundary`, `ProductEditModal`, `NotificationProvider`, `DefaultLayout`, `AuthGuard`, `SidebarProvider`. | ||
|
|
||
| ## Summary | ||
|
|
||
| | Kind | Style | | ||
| |------|--------| | ||
| | React component / returns JSX | `function Name()` | | ||
| | Everything else (utils, API, handlers, hooks, mappers) | `const name = () =>` | | ||
|
|
||
| Preserve existing explicit return types and type annotations when converting. |
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| /** | ||
| * Cloudinary delivery URL transforms for outbound image URLs. | ||
| * All Cloudinary image URLs must include format optimization (f_auto), | ||
| * quality adjustment (q_auto), and explicit dimensions with crop mode. | ||
| */ | ||
|
|
||
| const CLOUDINARY_UPLOAD_PREFIX = 'res.cloudinary.com/'; | ||
| const UPLOAD_PATH = '/image/upload/'; | ||
| const TRANSFORMS = 'f_auto,q_auto,w_800,h_600,c_fill'; | ||
|
|
||
| /** | ||
| * Applies standard delivery transforms to a Cloudinary image URL. | ||
| * Inserts f_auto, q_auto, and explicit dimensions (w_800, h_600, c_fill). | ||
| * Non-Cloudinary URLs are returned unchanged. | ||
| */ | ||
| export const toOptimizedImageUrl = (url: string | null | undefined): string | undefined => { | ||
| if (url == null || url === '') return undefined; | ||
| if (!url.includes(CLOUDINARY_UPLOAD_PREFIX) || !url.includes(UPLOAD_PATH)) return url; | ||
| const insert = `${UPLOAD_PATH}${TRANSFORMS}/`; | ||
| const afterUpload = url.indexOf(UPLOAD_PATH) + UPLOAD_PATH.length; | ||
| const alreadyHasTransforms = url.slice(afterUpload, afterUpload + 20).includes('f_auto'); | ||
| if (alreadyHasTransforms) return url; | ||
| return url.replace(UPLOAD_PATH, insert); | ||
| }; |
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
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
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
For hono v4.6.0 and@hono/zod-openapiv0.19.10, what is the recommended way to type handlers soapp.openapi(route, handler)works withoutas unknown as RouteHandler? Does annotating handlers asPromiseeraseTypedResponseinference fromc.json()/c.body()?💡 Result:
1) Typing handlers so
app.openapi(route, handler)accepts them (no cast)For
@hono/zod-openapi, the intended handler type isRouteHandler<R>, which is defined to returnMaybePromise<RouteConfigToTypedResponse<R>>(and only allows plainResponsein some cases). So the “recommended” way is to type-check your handler againstRouteHandler<typeof route>. (github.com)Best pattern (keeps inference, avoids widening):
You can also annotate directly:
But
satisfiesis usually nicer because it checks compatibility without forcing the function’s inferred return type to widen.2) Does
Promise<Response>annotation eraseTypedResponseinference?Yes—at the handler boundary.
In Hono’s types,
TypedResponse<T, U, F>is the marker that carries response-body/status/format type info. (jsr.io) If you annotate your handler asPromise<Response>(orResponse), TypeScript will accept returningc.json()(because it’s still aResponse), but it will widen the function’s return type toResponse, soapp.openapi(...)can no longer “see” theTypedResponseinformation for that handler’s output typing.So: don’t annotate OpenAPI handlers as
Promise<Response>; useRouteHandler<typeof route>(orsatisfiesit) to preserve theTypedResponse-based inference. (github.com)Citations:
🏁 Script executed:
Repository: sam821203/SalesOps
Length of output: 3043
🏁 Script executed:
Repository: sam821203/SalesOps
Length of output: 231
🏁 Script executed:
Repository: sam821203/SalesOps
Length of output: 303
🏁 Script executed:
Repository: sam821203/SalesOps
Length of output: 470
🏁 Script executed:
Repository: sam821203/SalesOps
Length of output: 3390
Refactor all handlers to use
RouteHandler<typeof route>instead ofPromise<Response>annotations.Every handler across the codebase is explicitly annotated as
Promise<Response>orResponse, which erases theTypedResponseinference that@hono/zod-openapineeds. This is why the castsas unknown as RouteHandler<...>are required in app.ts—they work around a type mismatch created by the controller annotations. The casts disable compile-time verification: mismatched input schemas or undocumented response shapes will still register cleanly.Fix by removing
Promise<Response>annotations from all handlers and usingRouteHandler<typeof route>instead (orsatisfies RouteHandler<typeof route>). This preservesTypedResponseinference fromc.json()/c.body()and allows the route chain to stay type-checked end to end.Affected files:
apps/api/src/modules/health/health.controller.ts(2 handlers)apps/api/src/modules/ecommerce/sku-price-history.controller.ts(3 handlers)apps/api/src/modules/ecommerce/inventory-adjustment.controller.ts(3 handlers)apps/api/src/modules/ecommerce/product.controller.ts(5 handlers)apps/api/src/modules/user/user.controller.ts(6 handlers)apps/api/src/modules/upload/upload.controller.ts(1 handler)apps/api/src/common/filters/error-handler.ts(1 handler)🤖 Prompt for AI Agents