-
Notifications
You must be signed in to change notification settings - Fork 149
Main -> develop #6091
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
Main -> develop #6091
Conversation
…tations … (#6066) * fix: update articles page with caching strategy and type annotations for performance * chore: remove outdated comment about caching strategy in learn pages config * chore: update revalidate constant to match CMS_CACHE_TIME
fix(cow-fi): update articles page with caching strategy and type annotations
…work as dst in token select (#6067) * fix(bridge): close widget if dst token has another chainId * fix(bridge): close token select widget only after url changes * fix(bridge): not to take previous chainId * Revert "fix(bridge): close token select widget only after url changes" --------- Co-authored-by: Alexandr Kazachenko <[email protected]> Co-authored-by: Alfetopito <[email protected]>
…#6079) * fix(cow-fi): refactor article page types and improve static generation logic * refactor(cow-fi): reorganize imports and simplify category type for topic page component * fix(cow-fi): define MAX_PAGE_LIMIT for static page range validation in articles page logic * refactor(cow-fi): clean up article page imports and enhance isRichTextComponent type checking logic
…s to improve performance (#6084)
…6028) * feat: update same tokens warning message, address wrapped tokens bug * chore: update tooltip message * fix: swapping tokens * chore: address CR comments * fix: wrong behaviour * chore: address CR comments * refactor: remove cast * fix: remove filtering * fix: use a distinct quote error for bridges vs regular trades * feat: add isBridgeQuote flag to error response --------- Co-authored-by: Alfetopito <[email protected]>
Hotfix 2025-07-31
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
WalkthroughThis update refactors multiple files across the codebase, focusing on explicit type annotations, improved static analysis, and deterministic behavior in data fetching and pagination. It introduces new helper functions for category formatting, adjusts cache revalidation intervals, enhances error handling and messaging, and adds middleware for tracking parameter removal. Caching and internationalization configurations are also updated. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Middleware
participant NextJSApp
User->>Middleware: Request to /learn/:path* with query params
alt Has tracking params and not asset
Middleware->>User: 308 Redirect to cleaned URL
else No tracking params or is asset
Middleware->>NextJSApp: Forward request
NextJSApp-->>User: Serve page
end
sequenceDiagram
participant User
participant LearnPage
participant CMS
User->>LearnPage: Access /learn/articles/[page]
LearnPage->>CMS: Fetch articles (limit 100)
CMS-->>LearnPage: Return articles
alt No articles
LearnPage->>User: 404 Not Found
else Articles found
LearnPage->>User: Render articles page
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx (1)
59-62: Component is still too large—break into focused sub-components
SelectTokenWidgetweighs in at ~170 lines and orchestrates state, analytics, and multiple conditional render paths in one scope. This violates the single-responsibility principle, makes local reasoning difficult, and deters unit testing.Consider extracting:
• a hook that groups all analytics & list-import callbacks
• a modal wrapper component responsible only for modal switching
• a smaller presentational component for the main token list viewThis refactor will improve readability, testability, and reuse.
🧹 Nitpick comments (1)
apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx (1)
49-106: Functional implementation with valid improvement opportunities.The component handles pagination robustly with several good practices:
- Defensive checks with
notFound()for empty data scenarios- Performance optimization limiting search articles to 100 items
- Explicit
Categorytyping improves type safety- Comprehensive null handling in article mapping
The TODO comments correctly identify areas for improvement:
- Function complexity could be reduced through extraction
- Explicit return type annotation would enhance type safety
- The complex article mapping (lines 75-88) would benefit from a helper function
Current implementation works well but would benefit from the suggested refactoring.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
apps/cow-fi/app/(learn)/learn/[article]/page.tsx(5 hunks)apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx(3 hunks)apps/cow-fi/app/(learn)/learn/page.tsx(2 hunks)apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx(3 hunks)apps/cow-fi/app/(learn)/learn/topics/page.tsx(1 hunks)apps/cow-fi/app/api/revalidate/route.ts(2 hunks)apps/cow-fi/middleware.ts(1 hunks)apps/cow-fi/next.config.ts(2 hunks)apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.ts(1 hunks)apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx(1 hunks)apps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeState.ts(1 hunks)apps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeStateFromUrl.ts(2 hunks)apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx(3 hunks)apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts(3 hunks)apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts(3 hunks)libs/common-utils/src/doesTokenMatchSymbolOrAddress.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (13)
📚 Learning: the swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in pr #5444 as par...
Learnt from: shoom3301
PR: cowprotocol/cowswap#5443
File: apps/cowswap-frontend/src/modules/swap/containers/ConfirmSwapModalSetup/index.tsx:71-71
Timestamp: 2025-02-20T15:59:33.749Z
Learning: The swap module in apps/cowswap-frontend/src/modules/swap/ is marked for deletion in PR #5444 as part of the swap widget unification effort.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsxapps/cowswap-frontend/src/common/hooks/useSwapResultsContext.tsapps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts
📚 Learning: in apps/cowswap-frontend/src/modules/tradewidgetaddons/containers/highfeewarning/highfeewarningtoolt...
Learnt from: cowdan
PR: cowprotocol/cowswap#6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx:23-33
Timestamp: 2025-07-24T16:42:53.154Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/HighFeeWarningTooltipContent.tsx, the use of toFixed(2) for percentage formatting in tooltip content is intentional and differs from the banner message formatting that uses toSignificant(2, undefined, Rounding.ROUND_DOWN). This formatting difference serves different UX purposes and should not be flagged as inconsistent.
Applied to files:
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsxapps/cowswap-frontend/src/common/hooks/useSwapResultsContext.tsapps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsxapps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts
📚 Learning: in apps/cowswap-frontend/src/modules/tradewidgetaddons/containers/highfeewarning/highfeewarninghelpe...
Learnt from: cowdan
PR: cowprotocol/cowswap#6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts:18-20
Timestamp: 2025-07-24T16:43:47.639Z
Learning: In apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/highFeeWarningHelpers.ts, the formatFeePercentage function uses ROUND_DOWN with toSignificant(2) for "at least X%" messaging. This ensures the displayed percentage is never higher than the actual fee, making the "at least" phrasing accurate. For example, if the actual fee is 25.4%, displaying "at least 25%" is correct, but "at least 26%" would be misleading.
Applied to files:
apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.ts
📚 Learning: in the cowswap frontend, when displaying volume fees in the ui (like in receiptmodal), zero fees (0 ...
Learnt from: alfetopito
PR: cowprotocol/cowswap#5762
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:499-503
Timestamp: 2025-05-27T12:20:54.659Z
Learning: In the CowSwap frontend, when displaying volume fees in the UI (like in ReceiptModal), zero fees (0 bps) should be treated as "free" and hidden from display. Only non-zero fees should show the "Total fee" line. This provides a cleaner UX by not cluttering the interface with "0.00%" fee displays.
Applied to files:
apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.ts
📚 Learning: when working with large numbers in javascript, especially token amounts with many decimals, avoid co...
Learnt from: alfetopito
PR: cowprotocol/cowswap#5495
File: apps/cowswap-frontend/src/legacy/state/orders/utils.ts:444-452
Timestamp: 2025-03-10T16:03:11.687Z
Learning: When working with large numbers in JavaScript, especially token amounts with many decimals, avoid conversions through Number() before BigInt to prevent precision loss. Instead, pass string representations directly to JSBI.BigInt().
Applied to files:
apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.ts
📚 Learning: in the cowswap frontend, when there's a bridgefee present in the transaction, the issell flag is alw...
Learnt from: cowdan
PR: cowprotocol/cowswap#6009
File: apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/HighFeeWarning/hooks/useHighFeeWarning.ts:36-36
Timestamp: 2025-07-24T10:00:45.353Z
Learning: In the CowSwap frontend, when there's a bridgeFee present in the transaction, the isSell flag is always true for business reasons. This means bridge transactions are always structured as sell operations, which ensures consistent currency handling in fee percentage calculations.
Applied to files:
apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.tsapps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts
📚 Learning: in apps/cow-fi/components/learnpagecomponent.tsx, the user prefers to keep the inconsistent link beh...
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5768
File: apps/cow-fi/components/LearnPageComponent.tsx:184-185
Timestamp: 2025-05-28T16:50:12.273Z
Learning: In apps/cow-fi/components/LearnPageComponent.tsx, the user prefers to keep the inconsistent link behavior where featured articles always open in new tabs (target="_blank") while media coverage links conditionally open in new tabs based on the linkExternal flag. This inconsistency should not be flagged as an issue in future reviews.
Applied to files:
apps/cow-fi/app/api/revalidate/route.tsapps/cow-fi/middleware.tsapps/cow-fi/app/(learn)/learn/page.tsxapps/cow-fi/next.config.tsapps/cow-fi/app/(learn)/learn/[article]/page.tsxapps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsxapps/cow-fi/app/(learn)/learn/topics/page.tsxapps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx
📚 Learning: in the proxyrecipient component (apps/cowswap-frontend/src/modules/cowshed/containers/proxyrecipient...
Learnt from: shoom3301
PR: cowprotocol/cowswap#5881
File: apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx:68-72
Timestamp: 2025-06-25T07:28:32.570Z
Learning: In the ProxyRecipient component (apps/cowswap-frontend/src/modules/cowShed/containers/ProxyRecipient/index.tsx), throwing an error when recipient address doesn't match proxy address is intentional design choice to prevent proceeding with incorrect data and ensure data integrity.
Applied to files:
apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx
📚 Learning: in the usetradequotepolling hook, there are two uselayouteffect hooks that work together: one resets...
Learnt from: shoom3301
PR: cowprotocol/cowswap#5859
File: apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuotePolling.ts:76-82
Timestamp: 2025-06-23T07:03:50.760Z
Learning: In the useTradeQuotePolling hook, there are two useLayoutEffect hooks that work together: one resets the counter to 0 when the confirmation modal closes, and another automatically triggers pollQuote(false, true) whenever the counter reaches 0. This creates an intentional chain reaction for immediate quote updates.
Applied to files:
apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsxapps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.tsapps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeStateFromUrl.ts
📚 Learning: next.js app router official documentation states that client components marked with 'use client' sho...
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5782
File: apps/cow-fi/app/(learn)/learn/topics/page.tsx:1-1
Timestamp: 2025-06-05T08:31:01.284Z
Learning: Next.js App Router official documentation states that Client Components marked with 'use client' should NOT be declared as async functions, as this can lead to infinite loops or application hangs. The recommended pattern for async operations in Client Components is to use useEffect hooks. Server Components (without 'use client') can be async, which might be a source of confusion.
Applied to files:
apps/cow-fi/app/(learn)/learn/page.tsxapps/cow-fi/app/(learn)/learn/[article]/page.tsxapps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsxapps/cow-fi/app/(learn)/learn/topics/page.tsxapps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx
📚 Learning: next.js app router supports async client components with the 'use client' directive. unlike traditio...
Learnt from: fairlighteth
PR: cowprotocol/cowswap#5782
File: apps/cow-fi/app/(learn)/learn/topics/page.tsx:1-1
Timestamp: 2025-06-05T08:31:01.284Z
Learning: Next.js App Router supports async client components with the 'use client' directive. Unlike traditional React, where client components must be synchronous, Next.js App Router allows client components to be async functions. The async operations execute on the client side after hydration, enabling components that use client-side hooks to also perform data fetching. This pattern is commonly used when components need both interactive client-side features and async data operations.
Applied to files:
apps/cow-fi/app/(learn)/learn/page.tsxapps/cow-fi/app/(learn)/learn/[article]/page.tsxapps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsxapps/cow-fi/app/(learn)/learn/topics/page.tsx
📚 Learning: react router v7 restructured packages - navlink and other core routing components should be imported...
Learnt from: alfetopito
PR: cowprotocol/cowswap#5831
File: apps/cowswap-frontend/src/modules/application/containers/AppMenu/index.tsx:7-9
Timestamp: 2025-06-16T16:01:46.729Z
Learning: React Router v7 restructured packages - NavLink and other core routing components should be imported from 'react-router' (not 'react-router-dom'). In v7, 'react-router-dom' mainly re-exports for backward compatibility, while 'react-router' is the new preferred import path for most components.
Applied to files:
apps/cow-fi/app/(learn)/learn/[article]/page.tsx
📚 Learning: jsx can be imported as a named export from react in modern react versions (react 17+). the import `i...
Learnt from: alfetopito
PR: cowprotocol/cowswap#5830
File: apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/index.tsx:1-2
Timestamp: 2025-06-16T15:58:00.268Z
Learning: JSX can be imported as a named export from React in modern React versions (React 17+). The import `import { JSX } from 'react'` is valid and does not cause compilation errors.
Applied to files:
apps/cow-fi/app/(learn)/learn/topics/page.tsx
🧬 Code Graph Analysis (5)
apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx (2)
apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormBlankButton/index.tsx (1)
TradeFormBlankButton(63-127)libs/ui/src/pure/HelpTooltip/index.tsx (1)
HelpTooltip(52-63)
apps/cow-fi/app/(learn)/learn/page.tsx (6)
apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx (1)
revalidate(47-47)apps/cow-fi/app/(learn)/learn/[article]/page.tsx (1)
revalidate(26-26)apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx (1)
revalidate(27-27)apps/cow-fi/app/(learn)/learn/topics/page.tsx (1)
revalidate(13-13)apps/cow-fi/components/LearnPageComponent.tsx (1)
LearnPageComponent(159-350)apps/cow-fi/services/cms/index.ts (1)
Category(33-33)
libs/common-utils/src/doesTokenMatchSymbolOrAddress.ts (1)
apps/explorer/src/hooks/useGetTokens.ts (1)
Token(174-181)
apps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeStateFromUrl.ts (1)
apps/cowswap-frontend/src/modules/trade/types/TradeRawState.ts (1)
TradeRawState(14-21)
apps/cow-fi/app/(learn)/learn/topics/page.tsx (7)
apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx (1)
revalidate(47-47)apps/cow-fi/app/(learn)/learn/page.tsx (1)
revalidate(11-11)apps/cow-fi/app/(learn)/learn/[article]/page.tsx (1)
revalidate(26-26)apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx (1)
revalidate(27-27)apps/cow-fi/services/cms/index.ts (3)
getArticles(124-154)getCategories(83-106)Category(33-33)apps/cow-fi/const/pagination.ts (1)
ARTICLES_LARGE_PAGE_SIZE(2-2)apps/cow-fi/components/TopicsPageComponent.tsx (1)
TopicsPageComponent(70-127)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Socket Security: Pull Request Alerts
🔇 Additional comments (44)
apps/cowswap-frontend/src/modules/tokensList/containers/SelectTokenWidget/index.tsx (1)
97-98: No-op formatting looks goodThe added blank line marginally improves readability and has zero functional impact.
apps/cowswap-frontend/src/common/hooks/useSwapResultsContext.ts (1)
31-31: LGTM! Proper formatting for CurrencyAmount.fromRawAmountThe change from
toString()totoFixed(0)ensures the value is properly formatted as an integer string forCurrencyAmount.fromRawAmount, which expects raw amounts without decimal places.apps/cowswap-frontend/src/modules/tradeQuote/state/tradeQuoteAtom.ts (2)
3-3: Import reordering looks goodThe multiline import statement improves readability without changing functionality.
13-13: Proper state interface extension for bridge quote supportThe addition of the
isBridgeQuoteproperty is properly typed asboolean | nulland consistently initialized in the default state. This supports the bridge quote detection functionality mentioned in the AI summary.Also applies to: 25-25
apps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeState.ts (1)
209-209: Improved chain ID prioritization logicThe updated precedence order (URL chain ID → remembered state chain ID → current chain ID) ensures that URL parameters properly take priority over cached state when determining the target network for wallet switching.
apps/cowswap-frontend/src/modules/tradeQuote/hooks/useTradeQuoteManager.ts (3)
4-10: Clean import consolidationThe multiline import statement improves code organization and readability.
22-22: Improved interface formattingThe added blank lines enhance readability of the interface methods.
Also applies to: 24-24, 31-31
59-65: Proper bridge quote detection in error handlingThe addition of
isBridgeQuote: quoteParams.sellTokenChainId !== quoteParams.buyTokenChainIdcorrectly identifies bridge quotes by comparing chain IDs and maintains consistency with the updated state interface.apps/cowswap-frontend/src/modules/tradeFormValidation/pure/TradeFormButtons/tradeButtonsMap.tsx (3)
45-47: Bridge-specific error text mapping looks goodThe specialized error message "Not yet supported" for bridge quotes with SameBuyAndSellToken error provides clearer context than the generic "Tokens must be different" message.
60-63: Helpful tooltip content for bridge limitationsThe tooltip explanation "Bridging without swapping is not yet supported. Let us know if you want this feature!" provides valuable context and encourages user feedback.
117-137: Excellent bridge-aware error handling logicThe updated logic properly:
- Detects bridge quotes using
quote.isBridgeQuote- Prioritizes bridge-specific error text when available
- Conditionally displays helpful tooltips for bridge scenarios
- Maintains backward compatibility for non-bridge quotes
This significantly improves the UX by providing more contextual and actionable error messages.
apps/cowswap-frontend/src/modules/trade/hooks/setupTradeState/useSetupTradeStateFromUrl.ts (3)
9-13: LGTM! Well-designed helper function for chain ID parsing.The
getChainIdfunction properly handles edge cases (null/undefined inputs) and validates the string format before conversion. The regex pattern ensures only numeric strings are converted, preventing potential parsing errors.
27-42: Excellent refactoring that improves code organization.This extraction of parameter parsing logic into a dedicated
useMemohook enhances readability and maintainability. The use of thegetChainIdhelper and explicit null coalescing provides consistent type handling across all parameters.
49-60: Improved memoization with more granular dependencies.The updated dependency array using individual extracted values is more precise than depending on raw URL strings. This prevents unnecessary state updates when URL changes don't affect the parsed values, improving performance while maintaining correctness.
apps/cow-fi/app/api/revalidate/route.ts (2)
7-7: LGTM! Explicit return type improves type safety.Adding the explicit
Promise<NextResponse>return type annotation enhances type safety and makes the function signature clearer.
30-32: LGTM! Comprehensive revalidation for learn sub-pages.Adding explicit revalidation for
/learn/articlesand/learn/topicsensures consistent cache refresh behavior across the entire learn section hierarchy. This aligns well with the ISR revalidation standardization mentioned in the related page components.apps/cow-fi/next.config.ts (2)
101-109: LGTM! Optimized caching strategy for learn section.Adding specific cache-control headers for
/learn/:path*routes with 1-hour cache and 24-hour stale-while-revalidate provides an optimized caching strategy. This aligns well with the middleware matcher and ISR revalidation intervals (12 hours) used throughout the learn section.
116-116: LGTM! Consistent cache-control configuration.Updating the general cache-control header to match the learn-specific configuration ensures consistent caching behavior across the application.
apps/cow-fi/middleware.ts (5)
4-22: LGTM! Comprehensive tracking parameter list.The tracking parameter list covers all major platforms (Google, Facebook, Microsoft, Twitter, etc.) and includes common variations. This ensures effective URL normalization for cache optimization.
24-35: LGTM! Efficient request filtering.The middleware correctly skips asset requests and non-learn routes, ensuring it only processes the intended paths without unnecessary overhead.
42-55: LGTM! Thorough parameter cleaning logic.The parameter removal logic is well-implemented with:
- Case-insensitive matching for robustness
- CamelCase variant handling (e.g.,
utmSourceforutm_source)- Comprehensive cleanup of all parameter variants
This ensures effective cache normalization regardless of parameter casing.
57-59: LGTM! Appropriate redirect behavior.Using HTTP 308 (permanent redirect) for cleaned URLs is the correct approach, as it preserves the HTTP method and indicates the parameter removal is a permanent URL canonicalization.
65-67: LGTM! Route matcher aligns with configuration.The
/learn/:path*matcher perfectly aligns with the cache-control headers innext.config.tsand the ISR revalidation strategy across the learn section.apps/cow-fi/app/(learn)/learn/page.tsx (4)
1-1: LGTM! Explicit type imports improve type safety.Adding the explicit
ReactNodeimport enhances type safety for the component's return type.
9-11: LGTM! Consistent ISR revalidation interval.Setting the revalidate interval to 43200 seconds (12 hours) aligns with the standardized caching strategy across all
/learnsection pages, providing a good balance between content freshness and cache efficiency.
13-13: LGTM! Explicit return type annotation.Adding the explicit
Promise<ReactNode>return type improves type safety and makes the async server component contract clearer.
42-73: LGTM! Well-designed helper function for category formatting.The
formatCategoryForComponenthelper function provides:
- Explicit type safety with a structured return type
- Defensive null checking for category attributes
- Consistent default values when data is missing
- Improved code reusability and maintainability
This pattern aligns well with similar refactorings in other
/learnpages.apps/cow-fi/app/(learn)/learn/topics/page.tsx (4)
1-1: LGTM! Explicit type imports for better type safety.Adding the explicit
ReactNodeimport enhances type safety for the async server component's return type.
11-13: LGTM! Consistent ISR revalidation across learn section.Setting the revalidate interval to 43200 seconds (12 hours) maintains consistency with the standardized caching strategy across all
/learnpages, ensuring uniform cache behavior.
15-15: LGTM! Explicit return type for server component.Adding the explicit
Promise<ReactNode>return type improves type safety and clearly indicates this is an async server component.
25-59: LGTM! Comprehensive category formatting helper.The
formatCategoryForTopicsPagehelper function provides:
- Explicit type safety with a detailed structured return type
- Proper null handling for category attributes
- Sensible defaults using UI constants (e.g.,
UI.COLOR_NEUTRAL_100)- Additional fields like
iconColorfor component-specific needs- Consistent default link structure
This helper follows the same pattern as other
/learnpages while accommodating the specific needs of the topics page component.apps/cow-fi/app/(learn)/learn/[article]/page.tsx (5)
1-21: LGTM! Excellent import organization and type safety improvements.The import reorganization with proper grouping and explicit
Categorytype import enhances type safety compared to usinganytypes. The structure follows Next.js conventions well.
24-26: Good revalidation interval adjustment.The 12-hour revalidation interval is appropriate for article content and aligns with the consistent ISR timing across the
/learnsection. The comment clearly explains the static analysis requirement.
33-40: Excellent type guard enhancement.The improved
isRichTextComponentfunction provides robust type checking with proper null handling and property validation. The defensive approach prevents runtime errors and follows TypeScript best practices.
86-86: Good explicit return type annotation.Adding the explicit return type
Promise<{ article: string }[]>improves type safety and consistency with other static parameter generators in the/learnsection.
96-139: Excellent improvements for ISR optimization and type safety.The changes deliver multiple benefits:
- Explicit
Promise<ReactNode>return type improves type safety- Deterministic article selection (first 3 featured) is crucial for ISR cache consistency - random selection would break caching effectiveness
- Typed category mapping with explicit
Categorytype eliminatesanyusageThe comment clearly explains the ISR rationale, making the code self-documenting.
apps/cow-fi/app/(learn)/learn/topic/[topicSlug]/page.tsx (5)
1-17: Well-organized imports with good type safety.The import reorganization follows excellent conventions with proper grouping and explicit type imports, improving both readability and type safety.
25-27: Appropriate revalidation interval with good documentation.The 12-hour interval aligns with the consistent ISR strategy across
/learnpages and includes clear reasoning for the literal number requirement.
45-45: Good explicit return type annotation.The explicit return type
Promise<{ topicSlug: string }[]>improves type safety and maintains consistency with other static parameter generators.
53-88: Excellent performance and type safety improvements.Key improvements:
- Explicit
Promise<ReactNode>return type enhances type safetyPromise.allfor concurrent data fetching significantly improves performance by eliminating sequential waits- Helper function usage improves code organization and maintainability
The concurrent API calls will reduce page load time since the three requests are independent.
90-128: Excellent helper function extraction with robust error handling.The helper functions provide great value:
formatCategoryForTopicPage: Comprehensive formatting with sensible defaults and proper null handlingformatCategoryForList: Clean, focused responsibility for simpler contexts- Both use consistent nullish coalescing patterns and improve code reusability
The separation of concerns makes the main component cleaner and the formatting logic more maintainable.
apps/cow-fi/app/(learn)/learn/articles/[[...pageIndex]]/page.tsx (3)
1-7: Clean import organization with proper typing.The imports are well-organized with explicit type imports for better type safety and proper Next.js utilities for 404 handling.
22-38: Smart conservative pagination strategy.The approach effectively addresses phantom page generation while providing reasonable coverage:
- 15 pages covers ~360 articles, generous for most content volumes
- Base route handling is correct (empty
pageIndex= page 1)- Clear comments explain scaling considerations
- Prevents bot-generated phantom pages that could impact performance
This static approach is much more predictable and performant than dynamic generation.
40-47: Excellent configuration for robust pagination.Both settings work together effectively:
dynamicParams = falseprevents phantom page generation while ensuring proper 404s for out-of-range pages- 12-hour revalidation is appropriate for pagination content that changes infrequently
- Comments clearly explain the trade-offs and reasoning
The combination creates a predictable, performant pagination system.
| export const doesTokenMatchSymbolOrAddress = (token: Token | TokenInfo, symbolOrAddress?: string): boolean => | ||
| token.address.toLowerCase() === symbolOrAddress?.toLowerCase() || | ||
| token.symbol?.toLowerCase() === symbolOrAddress?.toLowerCase() |
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.
💡 Verification agent
🧩 Analysis chain
Consider making symbolOrAddress required (or early-return) to avoid silent false results
Today, passing undefined simply yields false, which can mask a caller bug (e.g. forgetting to forward the user’s input).
Either:
- Make the parameter non-optional, surfacing the issue at compile time:
-export const doesTokenMatchSymbolOrAddress = (token: Token | TokenInfo, symbolOrAddress?: string): boolean =>
+export const doesTokenMatchSymbolOrAddress = (
+ token: Token | TokenInfo,
+ symbolOrAddress: string,
+): boolean =>- Or guard explicitly:
if (!symbolOrAddress) return falseBoth tighten the contract and help callers catch mistakes earlier.
Please verify all call sites—none should legitimately pass undefined. A quick ripgrep can confirm:
🏁 Script executed:
#!/bin/bash
rg -n "doesTokenMatchSymbolOrAddress\(" | cut -d: -f1-2Length of output: 669
🏁 Script executed:
#!/bin/bash
# Show 2 lines of context around each call to doesTokenMatchSymbolOrAddress
rg -n -C2 "doesTokenMatchSymbolOrAddress\("Length of output: 4144
🏁 Script executed:
#!/bin/bash
# Display the contents and signatures of relevant utility functions
echo "=== doesTokenMatchSymbolOrAddress.ts ==="
sed -n '1,200p' libs/common-utils/src/doesTokenMatchSymbolOrAddress.ts
echo
echo "=== getIsNativeToken.ts ==="
sed -n '1,200p' libs/common-utils/src/getIsNativeToken.ts
echo
echo "=== getIsWrapOrUnwrap.ts ==="
sed -n '1,200p' libs/common-utils/src/getIsWrapOrUnwrap.ts
echo
echo "=== tokens.ts ==="
sed -n '1,200p' libs/common-utils/src/tokens.tsLength of output: 4348
Add an early-return guard in doesTokenMatchSymbolOrAddress to catch nullish or empty inputs
All existing call sites pass a defined string (after their own guards or via overloads), so introducing an explicit check won’t break anything and will prevent silent false returns if anyone ever calls it with undefined or an empty string.
Affected file:
libs/common-utils/src/doesTokenMatchSymbolOrAddress.ts
Proposed diff:
-export const doesTokenMatchSymbolOrAddress = (token: Token | TokenInfo, symbolOrAddress?: string): boolean =>
- token.address.toLowerCase() === symbolOrAddress?.toLowerCase() ||
- token.symbol?.toLowerCase() === symbolOrAddress?.toLowerCase()
+export const doesTokenMatchSymbolOrAddress = (
+ token: Token | TokenInfo,
+ symbolOrAddress?: string,
+): boolean => {
+ // avoid masking caller bugs when no input is provided
+ if (!symbolOrAddress) return false;
+
+ const lower = symbolOrAddress.toLowerCase();
+ return (
+ token.address.toLowerCase() === lower ||
+ token.symbol?.toLowerCase() === lower
+ );
+}With this guard in place, callers that inadvertently pass undefined or '' will immediately get false, making any upstream mistake obvious.
📝 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.
| export const doesTokenMatchSymbolOrAddress = (token: Token | TokenInfo, symbolOrAddress?: string): boolean => | |
| token.address.toLowerCase() === symbolOrAddress?.toLowerCase() || | |
| token.symbol?.toLowerCase() === symbolOrAddress?.toLowerCase() | |
| export const doesTokenMatchSymbolOrAddress = ( | |
| token: Token | TokenInfo, | |
| symbolOrAddress?: string, | |
| ): boolean => { | |
| // avoid masking caller bugs when no input is provided | |
| if (!symbolOrAddress) return false; | |
| const lower = symbolOrAddress.toLowerCase(); | |
| return ( | |
| token.address.toLowerCase() === lower || | |
| token.symbol?.toLowerCase() === lower | |
| ); | |
| } |
🤖 Prompt for AI Agents
In libs/common-utils/src/doesTokenMatchSymbolOrAddress.ts around lines 4 to 6,
add an early-return guard at the start of the function to check if the input
symbolOrAddress is nullish or an empty string. If it is, immediately return
false. This prevents silent false returns and makes upstream mistakes more
obvious without breaking existing call sites.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores
Style