chore: fix fast-refresh & add new linting rule #6742
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughExtensive code reorganization refactoring that extracts constants, types, utilities, and hooks from index files into dedicated modules. Adds i18n Provider component with dynamic locale activation, reorganizes test utilities, consolidates Telegram notification logic, and integrates ESLint React Refresh plugin to ensure Fast Refresh compatibility. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Areas requiring extra attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
kernelwhisperer
force-pushed
the
fix/fast-refresh
branch
from
2a86de4 to
7a7ff86
Compare
kernelwhisperer
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/cowswap-frontend/src/test-utils.tsx (1)
36-48: Add requiredlocaleprop to LanguageProvider.The
Providercomponent (aliased asLanguageProvider) requires alocaleprop of typeSupportedLocaleaccording to theProviderPropsinterface defined in i18n.tsx. However,LanguageProvideris used here without providing this required prop, which will cause a TypeScript compilation error and potential runtime issues.🔎 Proposed fix
+import { DEFAULT_LOCALE } from '@cowprotocol/common-const' + const WithProviders = ({ children }: { children?: ReactNode }): ReactNode => { return ( - <LanguageProvider> + <LanguageProvider locale={DEFAULT_LOCALE}> <MockedI18nProvider> <Provider store={cowSwapStore}> <Web3Provider selectedWallet={undefined}> <MockThemeProvider>{children}</MockThemeProvider> </Web3Provider> </Provider> </MockedI18nProvider> </LanguageProvider> ) }apps/cowswap-frontend/src/modules/notifications/containers/NotificationsList/index.tsx (1)
91-94: Missingkeyprop on fragment in map.The
keyprop is placed onNotificationsListWrapper(line 94) instead of the outermost element returned by the map. React requires the key on the outermost element. This will cause a React warning.🔎 Proposed fix
{groups?.map((group) => ( - <> + <React.Fragment key={group.date.getTime()}> <h4>{group.date.toLocaleString(i18n.locale, DATE_FORMAT_OPTION)}</h4> - <NotificationsListWrapper key={group.date.getTime()}> + <NotificationsListWrapper> {group.notifications.map(({ id, thumbnail, title, description, url }) => {And update the closing tag:
</NotificationsListWrapper> - </> + </React.Fragment> ))}
🧹 Nitpick comments (12)
apps/cowswap-frontend/src/modules/yield/updaters/VampireAttackUpdater.tsx (1)
32-32: Minor: Extraneous blank line.This blank line inside the reduce callback appears to be an unintentional formatting change. Consider removing it for consistency.
🔎 Suggested fix
const result = Object.keys(lpTokensWithBalances).reduce( // TODO: Reduce function complexity by extracting logic - (acc, tokenAddress) => {apps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/utils.ts (1)
1-6: LGTM! Correct clamping implementation.The border width calculation correctly clamps the computed value between minimum and maximum thresholds. The constants are appropriately kept as internal implementation details, and the logic is efficient.
💡 Optional: Add JSDoc for clarity
Consider adding JSDoc comments to document the purpose of the constants and function:
+/** + * Minimum border width in pixels + */ const BORDER_WIDTH_MIN = 1.5 +/** + * Maximum border width in pixels + */ const BORDER_WIDTH_MAX = 2.2 +/** + * Ratio of icon size to use for border width calculation + */ const BORDER_WIDTH_RATIO = 0.15 +/** + * Calculates border width based on icon size, clamped between min and max + * @param size - The size of the icon in pixels + * @returns Clamped border width in pixels + */ export const getBorderWidth = (size: number): number => Math.max(BORDER_WIDTH_MIN, Math.min(BORDER_WIDTH_MAX, size * BORDER_WIDTH_RATIO))apps/cowswap-frontend/src/modules/account/containers/Transaction/utils.ts (1)
1-42: Approve the Fast Refresh refactoring; consider adding a default case.The extraction of
determinePillColourinto a separate utils module correctly addresses the HMR/Fast Refresh issue by removing non-component exports from the component file.However, the switch statement lacks a default case. If the
ActivityStatusenum is extended in the future, this function will returnundefined, violating thestringreturn type and potentially causing runtime issues.🔎 Recommended: Add a default case
case ActivityStatus.LOADING: return PILL_COLOUR_MAP.LOADING + default: + // Fallback for unknown status + return ActivityState.OPEN }apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsx (1)
3-8: Clean extraction of constants to support Fast Refresh.The constants are correctly imported from the new dedicated module, and
PROCESS_IMAGE_WRAPPER_BG_COLORis re-exported to maintain backward compatibility. Note thatTopSections.tsxnow imports directly from./constants, so this re-export may only be needed for other consumers.Optional: Verify if re-export is still needed
If no other modules import
PROCESS_IMAGE_WRAPPER_BG_COLORfrom./ProgressImageWrapper, the re-export on line 8 could be removed to simplify the API surface:#!/bin/bash # Description: Check if any modules import PROCESS_IMAGE_WRAPPER_BG_COLOR from ProgressImageWrapper (not from constants) # Search for imports from ProgressImageWrapper that include PROCESS_IMAGE_WRAPPER_BG_COLOR rg -n "from ['\"].*ProgressImageWrapper['\"]" --type tsx --type ts -A 1 | rg "PROCESS_IMAGE_WRAPPER_BG_COLOR"apps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.ts (1)
5-9: LGTM! Hook implementation follows correct Jotai pattern.The hook correctly uses
useAtomValueto provide read-only access to the atom state, while the component retains write access viauseAtom. This dual-access pattern is valid and achieves the fast-refresh compatibility goal.The TODO comment about the missing return type annotation is consistent with the codebase style (also present in the component file).
Optional: Add explicit return type annotation
If you'd like to address the TODO, you can add an explicit return type:
-// TODO: Add proper return type annotation -// eslint-disable-next-line @typescript-eslint/explicit-function-return-type -export function useIsNoImpactWarningAccepted() { +export function useIsNoImpactWarningAccepted(): boolean { return useAtomValue(noImpactWarningAcceptedAtom) }This would remove the need for the eslint-disable comment, but the current implementation is acceptable given the codebase conventions.
apps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsx (1)
46-50: Verify intentional styling difference for FAILED state.Line 48 uses an unstyled
<SVG>component for the FAILED state, while line 49 uses<StyledRefundCompleteIcon>for REFUND_COMPLETE. This difference might be intentional for visual distinction, but please confirm that the lack of styling on the FAILED icon matches the design requirements.apps/cowswap-frontend/src/lib/dynamicActivate.ts (1)
21-24: Clarify the error handling comment.The comment on line 22 states "Do nothing" but the code immediately logs an error to the console. Consider updating the comment to reflect the actual behavior, such as "Log error and continue" or remove the comment entirely since the logging action is self-documenting.
🔎 Suggested fix
} catch (error) { - // Do nothing - console.error('Could not load locale file: ' + locale, error) + // Log error and continue gracefully + console.error('Could not load locale file: ' + locale, error) }apps/cowswap-frontend/src/modules/orderProgressBar/updaters/orderFillabilityUtils.ts (1)
8-48: Consider adding JSDoc documentation.While the function names and implementation are clear, adding JSDoc comments would improve maintainability by documenting the parameters, return values, and the dual-source logic for
computeUnfillableOrderIds.Example documentation
+/** + * Computes a list of unfillable order IDs from two sources: + * 1. Orders explicitly marked as unfillable (price-based) + * 2. Orders lacking sufficient balance or allowance + * + * @param marketOrders - Array of market orders to check + * @param pendingOrdersFillability - Map of order IDs to their fillability status + * @returns Deduplicated array of unfillable order IDs + */ export function computeUnfillableOrderIds( marketOrders: OrderLike[], pendingOrdersFillability: Record<string, OrderFillability | undefined>, ): string[] { // ... } +/** + * Identifies orders that were previously unfillable but have now become fillable. + * + * @param previous - Set of previously unfillable order IDs + * @param current - Set of currently unfillable order IDs + * @returns Array of order IDs that recovered (were unfillable, now fillable) + */ export function getNewlyFillableOrderIds(previous: Iterable<string>, current: Iterable<string>): string[] { // ... }apps/cowswap-frontend/src/cow-react/index.tsx (1)
44-44: Clarify the rationale for exporting these internal components.The PR objective is to fix HMR issues by extracting constants, types, and utilities to separate modules. However, exporting
Main,Web3ProviderInstance, andToastsexposes application-level implementation details as public API.These exports don't align with the typical pattern of extracting non-component exports for HMR compatibility. Please verify:
- Are these exports intentional and required for the HMR fix?
- Is there a specific use case that needs access to these components?
- Should these remain internal to the entry point module?
If these exports are not necessary, consider removing them to avoid exposing internal implementation details.
Also applies to: 77-77, 88-88
apps/cowswap-frontend/src/legacy/components/Markdown/markdownComponents.tsx (3)
27-33: Consider idempotency and null safety in the effect.The
useEffectwith an empty dependency array only sets theidon mount. If the heading text changes after mount, theidwon't update. Additionally,textContentcan returnnull, thoughheadingToIdshould handle this.This implementation is acceptable if heading content is static after mount, which is typical for Markdown rendering.
Alternative: Move id generation to render phase
If you want the id to track content changes or avoid effects:
function H2({ children }: { children: ReactNode }) { - const ref = useRef<HTMLHeadingElement>(null) - - useEffect(() => { - const content = ref.current - - if (!content) return - - content.id = headingToId(content.textContent) - }, []) - - return <h2 ref={ref}>{children}</h2> + // Extract text for id at render time + const textContent = typeof children === 'string' ? children : '' + const id = headingToId(textContent) + + return <h2 id={id}>{children}</h2> }Note: This simplified approach only works if
childrenis always a string.
8-10: Avoid!importantif possible.Using
!importantoverrides can lead to specificity issues and make styles harder to maintain. Consider increasing selector specificity or addressing the root cause of the style conflict.
12-12: TODO comments indicate incomplete work.The return type annotations are missing. While TypeScript can infer the return types, explicit annotations improve code clarity and catch errors earlier.
Would you like me to generate the proper return type annotations for these components?
Also applies to: 22-22
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (62)
apps/cowswap-frontend/src/common/constants/dateFormats.tsapps/cowswap-frontend/src/common/hooks/useBlockNumber/BlockNumberProvider.tsxapps/cowswap-frontend/src/common/hooks/useBlockNumber/context.tsapps/cowswap-frontend/src/common/hooks/useBlockNumber/index.tsapps/cowswap-frontend/src/common/hooks/useBlockNumber/useBlockNumber.tsapps/cowswap-frontend/src/common/hooks/useNavigate.tsapps/cowswap-frontend/src/common/updaters/LpBalancesAndAllowancesUpdater.tsxapps/cowswap-frontend/src/common/updaters/lpBalancesState.tsapps/cowswap-frontend/src/cow-react/index.tsxapps/cowswap-frontend/src/legacy/components/Badge/RangeBadge.tsxapps/cowswap-frontend/src/legacy/components/Badge/index.tsxapps/cowswap-frontend/src/legacy/components/Badge/types.tsapps/cowswap-frontend/src/legacy/components/Markdown/components.tsxapps/cowswap-frontend/src/legacy/components/Markdown/markdownComponents.tsxapps/cowswap-frontend/src/lib/dynamicActivate.tsapps/cowswap-frontend/src/lib/i18n.tsxapps/cowswap-frontend/src/modules/account/containers/AccountDetails/index.tsxapps/cowswap-frontend/src/modules/account/containers/Transaction/StatusDetails.tsxapps/cowswap-frontend/src/modules/account/containers/Transaction/index.tsxapps/cowswap-frontend/src/modules/account/containers/Transaction/utils.tsapps/cowswap-frontend/src/modules/accountProxy/index.tsxapps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.tsapps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.tsxapps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/StackedProtocolIcons.tsxapps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/styled.tsxapps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/utils.tsapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsxapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/index.tsxapps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/index.tsxapps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.tsapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/index.tsxapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsxapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.tsapps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsxapps/cowswap-frontend/src/modules/notifications/containers/NotificationsList/index.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.tsapps/cowswap-frontend/src/modules/orderProgressBar/pure/TopSections.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.test.tsapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/orderFillabilityUtils.tsapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.tsapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/pure/Settings/index.tsxapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/index.tsxapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/types.tsapps/cowswap-frontend/src/modules/tradeSlippage/index.tsxapps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsxapps/cowswap-frontend/src/modules/yield/updaters/VampireAttackUpdater.tsxapps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsxapps/cowswap-frontend/src/pages/Account/Tokens/types.tsapps/cowswap-frontend/src/test-utils.tsxapps/cowswap-frontend/src/test-utils/mocks.tsxapps/cowswap-frontend/src/theme/components.tsxapps/cowswap-frontend/src/theme/consts.tsxapps/cowswap-frontend/src/theme/index.tsxapps/cowswap-frontend/src/theme/types.tsapps/cowswap-frontend/src/theme/zIndex.tseslint.config.jspackage.json
💤 Files with no reviewable changes (5)
- apps/cowswap-frontend/src/modules/tradeWidgetAddons/containers/TradeRateDetails/index.tsx
- apps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/styled.tsx
- apps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.tsx
- apps/cowswap-frontend/src/common/hooks/useNavigate.ts
- apps/cowswap-frontend/src/modules/tradeSlippage/index.tsx
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2025-08-08T13:55:17.528Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/state/tokens/allTokensAtom.ts:78-78
Timestamp: 2025-08-08T13:55:17.528Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts (TypeScript/Jotai), the team prefers to wait for token lists to initialize (listsStatesListAtom non-empty) before returning tokens. No fallback to favorites/user-added/native tokens should be used when listsStatesList is empty.
Applied to files:
apps/cowswap-frontend/src/pages/Account/Tokens/types.tsapps/cowswap-frontend/src/common/updaters/lpBalancesState.tsapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.tsapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsx
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/account/containers/Transaction/StatusDetails.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/TopSections.tsxapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/types.tsapps/cowswap-frontend/src/modules/account/containers/Transaction/index.tsxapps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.tsapps/cowswap-frontend/src/modules/account/containers/AccountDetails/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.tsapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/index.tsxapps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.tsxapps/cowswap-frontend/src/legacy/components/Badge/index.tsxapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/index.tsxapps/cowswap-frontend/src/test-utils.tsxapps/cowswap-frontend/src/modules/trade/pure/Settings/index.tsxapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsxapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsxapps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/index.tsxapps/cowswap-frontend/src/modules/accountProxy/index.tsxapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.tsapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/index.tsxapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/StackedProtocolIcons.tsxapps/cowswap-frontend/src/cow-react/index.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsxapps/cowswap-frontend/src/theme/components.tsxapps/cowswap-frontend/src/theme/consts.tsxapps/cowswap-frontend/src/legacy/components/Markdown/components.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsxapps/cowswap-frontend/src/theme/index.tsxapps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsx
📚 Learning: 2025-08-08T13:56:18.009Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6125
File: libs/tokens/src/updaters/TokensListsUpdater/index.tsx:29-31
Timestamp: 2025-08-08T13:56:18.009Z
Learning: In libs/tokens/src/updaters/TokensListsUpdater/index.tsx, the project’s current Jotai version requires using `unstable_getOnInit` (not `getOnInit`) in atomWithStorage options; keep `{ unstable_getOnInit: true }` until Jotai is upgraded.
Applied to files:
apps/cowswap-frontend/src/common/updaters/lpBalancesState.tsapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.tsapps/cowswap-frontend/src/common/updaters/LpBalancesAndAllowancesUpdater.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsxapps/cowswap-frontend/src/modules/yield/updaters/VampireAttackUpdater.tsx
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 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/trade/pure/TradeWarning/types.tsapps/cowswap-frontend/src/modules/account/containers/AccountDetails/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/index.tsxapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/index.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsxapps/cowswap-frontend/src/modules/notifications/containers/NotificationsList/index.tsx
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 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/account/containers/Transaction/index.tsxapps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/index.tsxapps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.tsxapps/cowswap-frontend/src/common/updaters/LpBalancesAndAllowancesUpdater.tsxapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/index.tsxapps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsxapps/cowswap-frontend/src/modules/accountProxy/index.tsxapps/cowswap-frontend/src/modules/trade/pure/TradeWarning/index.tsxapps/cowswap-frontend/src/modules/trade/index.tsapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsxapps/cowswap-frontend/src/theme/consts.tsxapps/cowswap-frontend/src/legacy/components/Markdown/components.tsxapps/cowswap-frontend/src/theme/index.tsx
📚 Learning: 2025-09-25T08:49:32.256Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:62-67
Timestamp: 2025-09-25T08:49:32.256Z
Learning: In the cowswap-frontend codebase, when testing hooks that use multiple bridge providers, both providers are always properly mocked as complete jest.Mocked<BridgeProvider<BridgeQuoteResult>> objects with all required methods stubbed, ensuring no undefined returns that could break the hook logic.
Applied to files:
apps/cowswap-frontend/src/test-utils/mocks.tsxapps/cowswap-frontend/src/test-utils.tsx
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.ts
📚 Learning: 2025-06-23T07:03:50.760Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 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/limitOrders/containers/LimitOrdersWidget/index.tsxapps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsx
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsxapps/cowswap-frontend/src/modules/notifications/containers/NotificationsList/index.tsx
📚 Learning: 2025-09-25T08:48:53.495Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6299
File: apps/cowswap-frontend/src/entities/bridgeProvider/useBridgeSupportedNetworks.test.tsx:58-60
Timestamp: 2025-09-25T08:48:53.495Z
Learning: In the cowswap-frontend codebase, when writing SWR tests, the team prefers maximum test isolation by using `provider: () => new Map()` in SWRConfig wrappers, even if it recreates cache on every render, to ensure tests don't share any state.
Applied to files:
apps/cowswap-frontend/src/test-utils.tsx
📚 Learning: 2025-09-11T08:25:51.460Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 6234
File: libs/tokens/src/index.ts:1-4
Timestamp: 2025-09-11T08:25:51.460Z
Learning: In the cowprotocol/cowswap project, there is currently no SSR (Server-Side Rendering) support, so localStorage access at module import time does not cause SSR-related issues.
Applied to files:
apps/cowswap-frontend/src/test-utils.tsx
📚 Learning: 2025-06-25T07:28:32.570Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 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/accountProxy/index.tsx
📚 Learning: 2025-08-12T06:33:19.348Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 6137
File: libs/tokens/src/state/tokens/allTokensAtom.ts:34-65
Timestamp: 2025-08-12T06:33:19.348Z
Learning: In libs/tokens/src/state/tokens/allTokensAtom.ts, the parseTokenInfo() function returns a new instance of TokenInfo each time, making it safe to mutate properties like lpTokenProvider on the returned object without side effects.
Applied to files:
apps/cowswap-frontend/src/modules/yield/updaters/VampireAttackUpdater.tsx
📚 Learning: 2025-06-16T15:58:00.268Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 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/cowswap-frontend/src/theme/consts.tsxapps/cowswap-frontend/src/legacy/components/Markdown/components.tsx
📚 Learning: 2025-06-16T16:01:46.729Z
Learnt from: alfetopito
Repo: cowprotocol/cowswap PR: 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/cowswap-frontend/src/legacy/components/Markdown/components.tsx
🧬 Code graph analysis (10)
apps/cowswap-frontend/src/common/hooks/useBlockNumber/useBlockNumber.ts (2)
apps/cowswap-frontend/src/common/hooks/useBlockNumber/context.ts (1)
BlockNumberContext(11-11)apps/cowswap-frontend/src/common/hooks/useBlockNumber.tsx (1)
useBlockNumber(24-26)
apps/cowswap-frontend/src/lib/dynamicActivate.ts (2)
libs/common-const/src/locales.ts (2)
SupportedLocale(8-8)DEFAULT_LOCALE(10-10)jest.setup.ts (1)
i18n(51-51)
apps/cowswap-frontend/src/theme/types.ts (1)
apps/cowswap-frontend/src/theme/consts.tsx (1)
TextProps(3-3)
apps/cowswap-frontend/src/legacy/components/Markdown/markdownComponents.tsx (3)
libs/common-hooks/src/useTheme.ts (1)
styled(26-26)apps/cowswap-frontend/src/theme/components.tsx (1)
Link(30-32)apps/cowswap-frontend/src/legacy/components/Markdown/components.tsx (1)
markdownComponents(1-1)
apps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsx (1)
apps/cowswap-frontend/src/modules/trade/pure/UnlockWidgetScreen/index.tsx (1)
BulletListItem(14-17)
apps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.ts (1)
apps/cowswap-frontend/src/modules/trade/index.ts (1)
useIsNoImpactWarningAccepted(52-52)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (1)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.ts (1)
ConnectTelegramController(14-23)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.ts (4)
apps/cowswap-frontend/src/modules/notifications/hooks/useTgSubscription.tsx (1)
useTgSubscription(123-199)apps/cowswap-frontend/src/modules/notifications/utils/devTg.ts (1)
TelegramData(6-13)libs/wallet/src/api/hooks.ts (1)
useWalletInfo(24-26)apps/cowswap-frontend/src/modules/notifications/hooks/useTgAuthorization.ts (1)
useTgAuthorization(35-126)
apps/cowswap-frontend/src/cow-react/index.tsx (1)
apps/cowswap-frontend/src/theme/components.tsx (1)
Main(27-29)
apps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsx (1)
libs/ui/src/pure/Tooltip/index.tsx (1)
HoverTooltip(61-177)
⏰ 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). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (66)
apps/cowswap-frontend/src/common/updaters/lpBalancesState.ts (1)
1-3: LGTM!Clean extraction of the atom to its own module. This correctly addresses the react-refresh compatibility issue by separating the Jotai atom from the React component file.
apps/cowswap-frontend/src/modules/yield/updaters/VampireAttackUpdater.tsx (1)
12-12: LGTM!Import path correctly updated to reference the extracted atom from its new dedicated state module.
apps/cowswap-frontend/src/common/updaters/LpBalancesAndAllowancesUpdater.tsx (1)
1-12: LGTM!The refactor correctly:
- Removes the
atomimport from jotai (no longer needed locally)- Imports
areLpBalancesLoadedAtomfrom the new dedicated state module- Ensures this file now only exports React components, making it compatible with react-refresh
This follows the established pattern for fixing HMR issues as described in the PR objectives.
apps/cowswap-frontend/src/modules/trade/pure/Settings/index.tsx (1)
2-2: LGTM – Formatting improvements.The added blank lines improve readability by visually separating the export groups. Since this PR adds ESLint rules to enforce fast-refresh compatibility, any incompatible exports from the
./styledmodule should be caught automatically.Also applies to: 4-4
apps/cowswap-frontend/src/legacy/components/Badge/RangeBadge.tsx (1)
7-8: LGTM! Import separation fixes fast-refresh compatibility.The refactored imports correctly separate the component (default export) from the type definition, resolving HMR incompatibility issues mentioned in issue #6738. The change maintains functionality while enabling fast-refresh to work without full page reloads.
apps/cowswap-frontend/src/legacy/components/Badge/types.ts (1)
1-13: LGTM! Clean type separation for fast-refresh compatibility.The new types module successfully centralizes Badge type definitions, enabling the component to maintain fast-refresh compatibility. The enum values are well-defined, and the interface follows TypeScript conventions.
apps/cowswap-frontend/src/legacy/components/Badge/index.tsx (1)
6-10: LGTM! Proper type delegation maintains public API.The refactored import and re-export structure correctly delegates type declarations to the separate types module while maintaining backward compatibility. The re-export pattern ensures existing consumers can continue importing
BadgeVariantandBadgePropsfrom the Badge module without breaking changes.The use of
import typeon line 8 forDefaultThemeis a good TypeScript optimization for type-only imports.apps/cowswap-frontend/src/modules/application/containers/App/FaviconAnimationUpdater.test.ts (1)
31-31: LGTM! Import path updated to support HMR fix.The import now points to the dedicated
faviconAnimation/logicmodule, which aligns with the PR objective of extracting utilities from component files to resolve fast-refresh issues. The test logic remains unchanged, preserving coverage.apps/cowswap-frontend/src/modules/bridge/pure/ProtocolIcons/StackedProtocolIcons.tsx (1)
8-9: LGTM! Clean import refactor.The extraction of
getBorderWidthto a separate utils module aligns perfectly with the PR objective to fix fast-refresh issues by separating utility functions from component files. The refactor is minimal and preserves the existing behavior.apps/cowswap-frontend/src/modules/trade/pure/TradeWarning/types.ts (1)
1-18: LGTM! Clean extraction of types for fast-refresh compatibility.This refactoring correctly addresses the HMR/fast-refresh issue by separating type definitions from the component file. React Fast Refresh requires that component files should not mix component exports with non-component exports (types, constants, enums). The type definitions are complete and well-structured.
apps/cowswap-frontend/src/modules/trade/pure/TradeWarning/index.tsx (1)
8-10: LGTM! Correct import and re-export pattern.The changes correctly import types from the new dedicated module and re-export them to maintain backward compatibility. This ensures that consumers importing
TradeWarningTypeorTradeWarningPropsfrom this module will continue to work without modification.apps/cowswap-frontend/src/modules/account/containers/Transaction/StatusDetails.tsx (1)
24-24: LGTM! Import path correctly updated.The import has been properly updated to reference the new utils module location, supporting the Fast Refresh refactoring effort.
apps/cowswap-frontend/src/modules/account/containers/Transaction/index.tsx (1)
8-8: LGTM! Clean extraction of utility logic from component file.Removing the unused type imports and extracting the pill color determination logic to a separate utils module correctly addresses the Fast Refresh incompatibility. The Activity component doesn't directly use these types or the
determinePillColourfunction, so the removal is appropriate.apps/cowswap-frontend/src/modules/orderProgressBar/pure/TopSections.tsx (1)
17-18: LGTM! Import refactor supports Fast Refresh.The separation of the constant import from the component import aligns with React Fast Refresh requirements and resolves the HMR issues described in #6738.
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.ts (2)
5-8: Excellent type safety with utility types.The type definitions correctly leverage TypeScript utility types to ensure type-safe, exhaustive maps for all step names.
10-65: Complete and well-structured constant maps.All three styling maps are exhaustive, covering every step name with appropriate values. The use of CSS custom properties via
var()alongside hex colors is a valid pattern for theme-aware styling.package.json (1)
321-321: LGTM! Plugin dependency correctly added.The
eslint-plugin-react-refreshdevDependency is properly placed and versioned. The caret range (^0.4.5) follows standard semantic versioning practices for ESLint plugins and complements the configuration changes ineslint.config.js.eslint.config.js (1)
12-12: LGTM! React Refresh plugin correctly integrated.The plugin import and configuration are properly implemented following ESLint flat config standards. The rule targets JSX/TSX files (
**/*.{jsx,tsx}) at the 'warn' level, which is appropriate for gradual adoption without breaking builds.The rule configuration uses strict mode with no
allowConstantExportorallowExportNamesoptions, aligning with the approach of completely separating component and non-component exports to ensure reliable Fast Refresh.apps/cowswap-frontend/src/modules/accountProxy/index.tsx (1)
1-17: LGTM! Module API expansion looks good.The new component exports expand the accountProxy module's public API in a clean, organized manner. Since this is a barrel file that re-exports from separate source modules, it's compatible with fast-refresh as long as the individual source files (under
./containers/,./hooks/,./utils/) maintain proper separation of concerns—which aligns with this PR's objectives.The consistent formatting with blank lines between export groups improves readability.
apps/cowswap-frontend/src/pages/Account/Tokens/types.ts (1)
1-4: LGTM! Clean enum extraction for fast refresh compatibility.The enum is correctly defined and follows the PR's objective of extracting non-component exports from component files to resolve HMR issues.
apps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsx (1)
50-50: LGTM! Correct import of extracted enum.The import statement correctly references the new types module, aligning with the fast refresh compatibility fix.
apps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/index.tsx (2)
1-1: LGTM! Import refactor correctly supports the atom extraction.The import changes properly reflect the separation of the atom definition into a dedicated module. The component now imports only
useAtom(for read-write access) and sourcesnoImpactWarningAcceptedAtomfrom the new module, which aligns with the fast-refresh compatibility goals.Also applies to: 13-13
39-39: LGTM! Atom usage is correct.The component correctly uses
useAtomwith the imported atom to maintain both read and write access to the acceptance state. The refactor preserves the original behavior while improving fast-refresh compatibility.apps/cowswap-frontend/src/modules/trade/index.ts (1)
52-52: LGTM! Export path correctly updated.The export path update correctly reflects the hook's relocation to a dedicated module, maintaining the public API while supporting the fast-refresh refactor.
apps/cowswap-frontend/src/modules/trade/containers/NoImpactWarning/useIsNoImpactWarningAccepted.ts (1)
1-3: LGTM! Atom definition and export are correct.The public export of
noImpactWarningAcceptedAtomis intentional and necessary, as theNoImpactWarningcomponent imports it directly for read-write access viauseAtom. The simpleatom(false)initialization is appropriate for this use case.apps/cowswap-frontend/src/theme/types.ts (1)
1-3: LGTM!The TextProps type definition is clean and appropriate. Omitting 'css' from Rebass TextProps is reasonable since styled-components handles CSS separately, and the
overrideflag aligns with its usage in TextWrapper.apps/cowswap-frontend/src/theme/index.tsx (1)
4-5: LGTM!Replacing the wildcard export with explicit named exports
{ TextWrapper, ThemedText }aligns with the PR's objective to fix fast-refresh/HMR issues by creating a more controlled and predictable export surface.apps/cowswap-frontend/src/theme/zIndex.ts (1)
1-20: LGTM!The Z_INDEX enum and WIDGET_MAX_WIDTH constant are well-structured. The inclusion of deprecated values with clear naming provides a good migration path, and the hierarchical z-index values follow Bootstrap's recommended system.
apps/cowswap-frontend/src/theme/consts.tsx (1)
1-3: LGTM!Clean consolidation that re-exports from dedicated modules (zIndex.ts and types.ts). This separation of concerns aligns well with the PR's objective to fix fast-refresh issues.
apps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsx (6)
1-14: LGTM! Clean import structure.The imports are well-organized and appropriate for the module's purpose. The use of
styled-components/macrois correct for build-time optimization.
28-32: Clear documentation.The module-level comment effectively explains the centralization of status constants, helping future maintainers understand the refactoring.
34-43: Well-structured icon mapping.The reuse of
CloseIconfor both FAILED and REFUND_COMPLETE states is efficient, and the type-safe mapping ensures all status enum values are covered.
52-54: Clean alias pattern.Using a direct alias for
BridgeStatusIconsis more maintainable than spreading when the mappings are truly identical.
57-63: Proper i18n integration.The swap status titles correctly use Lingui's
msgmacro for internationalization, with consistent verb tense patterns across status states.
66-75: Excellent DRY principle application.Extracting
bridgeFailedTitleand reusing it for both FAILED and REFUND_COMPLETE states eliminates duplication and ensures consistency.apps/cowswap-frontend/src/modules/bridge/pure/StopStatus/index.tsx (1)
1-1: Excellent Fast Refresh refactoring.This re-export pattern successfully addresses the HMR issue by moving constants to a separate module while maintaining backward compatibility for existing consumers. The approach aligns perfectly with the PR's objective to fix Fast Refresh problems by separating non-component exports.
apps/cowswap-frontend/src/test-utils/mocks.tsx (1)
1-18: LGTM! Clean mock implementation for testing.The MockedConnector class provides a minimal, working mock for web3 connector testing. The TODO about the return type annotation for
getActions()is already flagged, and the implementation correctly extends the Connector interface with the required methods.apps/cowswap-frontend/src/test-utils.tsx (1)
56-59: Clean module organization.The re-export of mocks from
./test-utils/mocksand the usage ofmockedConnectorandmockedConnectorHooksproperly centralizes test utilities. This separation improves maintainability and follows good module organization patterns.apps/cowswap-frontend/src/lib/i18n.tsx (1)
18-39: Well-structured Provider component with clear separation of concerns.The Provider component properly:
- Delegates locale loading to the
dynamicActivatemodule- Handles the DEFAULT_LOCALE initialization edge case with clear documentation
- Uses appropriate error handling that logs failures but continues rendering
- Includes correct dependency array for the useEffect hook
The refactoring successfully centralizes i18n activation logic while maintaining the necessary runtime behavior.
apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.test.ts (1)
3-3: LGTM - Import path correctly updated.The import statement has been updated to reflect the new module structure where utilities were extracted to
orderFillabilityUtils.ts. Test logic remains unchanged, which confirms this is a clean refactoring.apps/cowswap-frontend/src/modules/orderProgressBar/updaters/OrderProgressEventsUpdater.tsx (1)
19-21: LGTM - Proper separation of utilities from component file.The refactoring correctly imports the extracted utilities from
orderFillabilityUtils.ts, which aligns with React Fast Refresh requirements. Component files should primarily export React components, and moving utility functions to a separate module prevents HMR invalidation issues.apps/cowswap-frontend/src/modules/orderProgressBar/updaters/orderFillabilityUtils.ts (3)
8-35: LGTM - Logic is sound and correctly implemented.The
computeUnfillableOrderIdsfunction correctly:
- Filters orders marked as unfillable by price checks
- Identifies orders lacking balance or allowance (accounting for permit bypass)
- Deduplicates using Set when orders are flagged by both mechanisms
37-48: LGTM - Efficient implementation for detecting recoveries.The function correctly identifies orders that were previously unfillable but are no longer in the current unfillable set, indicating they've become fillable again. The use of Set for lookup is appropriately efficient.
1-48: Refactoring verified and complete.All imports have been correctly updated. The functions
computeUnfillableOrderIdsandgetNewlyFillableOrderIdsare properly exported fromorderFillabilityUtils.tsand imported from the new location in bothOrderProgressEventsUpdater.tsxand its test file. No remaining imports from the old location exist.apps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/index.tsx (1)
12-13: LGTM! Clean separation of types from component.This refactoring correctly addresses the HMR issue by moving type definitions to a dedicated module. The pattern of re-exporting types while importing only what's needed for the component is idiomatic and aligns with react-refresh requirements.
apps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.ts (2)
8-23: LGTM! Well-documented transaction state type.The
TxStatetype is clearly structured with comprehensive documentation explaining the semantic meaning of undefined vs boolean values for each field. This makes the API intuitive and self-documenting.
25-58: LGTM! Comprehensive and well-structured interface.The
EthFlowStepperPropsinterface is thorough, with clear field names and helpful documentation. The separation of concerns (order state, creation tx, refund tx, cancellation tx) makes the API easy to understand and use.apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.ts (2)
14-23: Well-defined controller interface.The
ConnectTelegramControllerinterface is well-structured and provides a clear contract for the hook's return type. This separation enables better testability and follows the controller pattern used elsewhere in the codebase.
38-54: Script loading logic is clean.Good defensive check for existing scripts before appending, and proper use of
onloadcallback for state management.apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (2)
5-10: Clean controller pattern separation.Good refactor extracting the hook logic to a separate module and re-exporting for backward compatibility. This aligns with the two-layer architecture pattern used elsewhere in the codebase, where stateful logic is separated from presentation.
18-33: Component is now a pure presentation layer.The component correctly destructures the controller and passes props to
TelegramConnectionStatus. This makes the component easier to test and reason about.apps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsx (1)
87-89: Good usage of the controller pattern.The hook is called at the top level and the controller is passed to
ConnectTelegram, maintaining proper React patterns and enabling the component to remain stateless.apps/cowswap-frontend/src/modules/notifications/containers/NotificationsList/index.tsx (1)
10-10: Good extraction of shared constant.Moving
DATE_FORMAT_OPTIONto a shared constants file resolves the fast-refresh incompatibility issue and improves reusability.apps/cowswap-frontend/src/common/constants/dateFormats.ts (1)
1-3: LGTM! Clean constant extraction for HMR compatibility.The shared date format constant is correctly typed and will enable better HMR behavior by separating this non-component export from component files.
apps/cowswap-frontend/src/modules/account/containers/AccountDetails/index.tsx (2)
30-30: LGTM! Correct import of shared constant.The import correctly references the newly centralized
DATE_FORMAT_OPTIONconstant.
202-202: Migration verified—all references use the shared location.The constant has been successfully migrated to
common/constants/dateFormatsand all imports across the codebase (AccountDetails, NotificationsList) correctly reference the shared location. No remaining local exports or duplicate definitions found.apps/cowswap-frontend/src/common/hooks/useBlockNumber/index.ts (1)
1-2: LGTM! Clean barrel export pattern.The index barrel correctly establishes a unified public API for block number functionality, supporting better module organization and HMR compatibility.
apps/cowswap-frontend/src/common/hooks/useBlockNumber/useBlockNumber.ts (2)
1-3: LGTM! Correct imports.The imports are appropriate for the hook implementation.
5-13: LGTM! Proper sentinel-based provider check.The hook correctly validates that it's used within a
BlockNumberProviderby checking against the sentinel value, throwing a clear error if the provider is missing.apps/cowswap-frontend/src/common/hooks/useBlockNumber/BlockNumberProvider.tsx (2)
1-6: LGTM! Clean import refactor.The imports correctly reflect the new module structure, with
BlockNumberContextnow sourced from the dedicated context module.
10-65: LGTM! Provider implementation correctly refactored.The provider logic remains intact and correctly uses the externalized
BlockNumberContext, maintaining the same behavior while supporting better module organization.apps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsx (2)
1-4: LGTM!The imports are appropriate for the i18n usage pattern in this constants file.
21-27: LGTM!The
UNLOCK_SCREENobject correctly usesmsgfor translatable strings and a plain string for the URL.apps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/index.tsx (1)
7-8: LGTM! Correct i18n pattern for extracted constants.The refactor correctly:
- Uses
tmacro for inline translations- Imports constants from the new module
- Obtains
i18nviauseLingui()hook- Translates
MessageDescriptorobjects withi18n._()This pattern properly separates static content from component logic while maintaining proper i18n handling.
Note: The
UnlockWidgetScreencomponent must handlecontentas eitherMessageDescriptor(translating it withi18n._()) orReact.ReactNode(rendering directly), per theBulletListItemtype definition. Assuming the component follows this contract, the current usage is correct.Also applies to: 30-30, 123-123, 182-189
apps/cowswap-frontend/src/legacy/components/Markdown/components.tsx (1)
1-1: LGTM! Clean refactoring for fast-refresh compatibility.The re-export pattern correctly separates component definitions into a dedicated module while preserving the public API. This aligns with the PR objective to fix HMR issues by isolating component exports.
apps/cowswap-frontend/src/legacy/components/Markdown/markdownComponents.tsx (1)
13-13: Thereact-refresh/only-export-componentsdisable is justified and necessary.The react-markdown library requires a
componentsobject mapping element names to React components. ThemarkdownComponentsobject export ({ table: Table, h2: H2, a: LinkScrollableStyled }) is directly consumed by the react-markdowncomponentsprop in URLWarning. This object export pattern is mandated by the library's API, not an internal implementation choice. Exporting components directly would require the consuming code to rebuild the mapping object anyway, which would not improve fast-refresh behavior. The disable is appropriate here.Likely an incorrect or invalid review comment.
apps/cowswap-frontend/src/common/hooks/useBlockNumber/context.ts
Outdated
Show resolved
Hide resolved
|
|
||
| // TODO: Add proper return type annotation | ||
| // eslint-disable-next-line @typescript-eslint/explicit-function-return-type, react-refresh/only-export-components | ||
| function Table({ children }: { children: ReactElement }) { |
There was a problem hiding this comment.
Fix incorrect children type for Markdown components.
Both Table and H2 components type their children as ReactElement, which is too restrictive. Markdown renderers typically pass:
- For
H2: text strings like "My Heading" (type:string | ReactNode) - For
Table: multiple elements, fragments, or mixed content (type:ReactNode)
The current ReactElement type will cause TypeScript errors when these components receive string children or fragments from the Markdown renderer.
🔎 Proposed fix
+import { ReactNode, useEffect, useRef } from 'react'
-import { ReactElement, useEffect, useRef } from 'react'
-function Table({ children }: { children: ReactElement }) {
+function Table({ children }: { children: ReactNode }) {
-function H2({ children }: { children: ReactElement }) {
+function H2({ children }: { children: ReactNode }) {Also applies to: 24-24
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/legacy/components/Markdown/markdownComponents.tsx
around lines 14 and 24, the Table and H2 components currently type their
children as ReactElement which is too restrictive for Markdown renderers; change
both props to accept React.ReactNode (or string | React.ReactNode) instead,
update the function signatures accordingly, and ensure React is imported or the
React type is available so TypeScript allows strings, fragments, arrays and
mixed content as children.
| export enum SmartOrderStatus { | ||
| CREATING = 'CREATING', | ||
| CREATION_MINED = 'CREATED', | ||
| INDEXED = 'INDEXED', | ||
| FILLED = 'FILLED', | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for SmartOrderStatus enum usage and references to CREATION_MINED and CREATED in the codebase
cd /repo
# First, find the enum file to confirm location
find . -type f -name "*.ts" -path "*/ethFlow/*" | head -20
# Search for CREATION_MINED usage
echo "=== CREATION_MINED references ==="
rg -nP --type=ts --type=tsx "CREATION_MINED" | head -30
# Search for CREATED status references
echo "=== CREATED status references ==="
rg -nP --type=ts --type=tsx "status.*['\"]CREATED['\"]|SmartOrderStatus.*CREATED|['\"]CREATED['\"]" | head -30
# Search for references to SmartOrderStatus to understand usage context
echo "=== SmartOrderStatus references ==="
rg -nP --type=ts --type=tsx "SmartOrderStatus" | head -30Repository: cowprotocol/cowswap
Length of output: 1892
🏁 Script executed:
#!/bin/bash
# Search for enum file first and view its content
echo "=== Enum file content ==="
cat -n ./apps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.ts
echo ""
echo "=== Search for CREATION_MINED usage ==="
rg "CREATION_MINED" --type ts
echo ""
echo "=== Search for CREATED string value ==="
rg "CREATED" --type ts
echo ""
echo "=== Search for SmartOrderStatus usage ==="
rg "SmartOrderStatus" --type ts -A 3Repository: cowprotocol/cowswap
Length of output: 25599
🏁 Script executed:
#!/bin/bash
# Search for Order type definition and status field
echo "=== Order type definition ==="
rg "type Order|interface Order" --type ts -A 10 | head -50
echo ""
echo "=== Backend status or API response types ==="
rg "status.*=.*CREATED|CREATED.*status|order.*status" --type ts | grep -i "backend\|api\|response\|type" | head -20
echo ""
echo "=== Search for where the 'creating' status is used ==="
rg "status === 'creating'" --type ts -B 2 -A 2Repository: cowprotocol/cowswap
Length of output: 6701
🏁 Script executed:
#!/bin/bash
# Search for ethFlow Order/order type definition and status field
echo "=== EthFlowStepper index.tsx - getSmartOrderStatus function ==="
rg "function getSmartOrderStatus|const getSmartOrderStatus" --type ts -A 20 apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlowStepper/index.tsx
echo ""
echo "=== Search for Order type in ethFlow ==="
rg "type Order|interface Order" --type ts apps/cowswap-frontend/src/modules/ethFlow -A 15 | head -80
echo ""
echo "=== Search for where order.status is set ==="
rg "order\.status|status.*=" --type ts apps/cowswap-frontend/src/modules/ethFlow/types.ts -A 2 -B 2Repository: cowprotocol/cowswap
Length of output: 214
🏁 Script executed:
#!/bin/bash
# Look for the Order import and type definition in the container file
echo "=== Full EthFlowStepper container file ==="
cat -n apps/cowswap-frontend/src/modules/ethFlow/containers/EthFlowStepper/index.tsx | head -100
echo ""
echo "=== Search in ethFlow types.ts ==="
cat -n apps/cowswap-frontend/src/modules/ethFlow/types.ts | head -50
echo ""
echo "=== Search for ORDER_INDEXED_STATUSES ==="
rg "ORDER_INDEXED_STATUSES" --type ts -B 5 -A 5Repository: cowprotocol/cowswap
Length of output: 6857
🏁 Script executed:
#!/bin/bash
# Find the Order and OrderStatus definitions
echo "=== OrderStatus enum ==="
rg "enum OrderStatus|type OrderStatus" --type ts -A 15 legacy/state/orders/
echo ""
echo "=== Check where CREATED string is used in backend communication ==="
rg "CREATED" --type ts apps/cowswap-frontend/src -B 3 -A 3 | grep -A 3 -B 3 "api\|request\|response\|fetch"Repository: cowprotocol/cowswap
Length of output: 234
🏁 Script executed:
#!/bin/bash
# Find the correct path for Order and OrderStatus
echo "=== Find Order/OrderStatus definitions ==="
find . -type f -name "*.ts" | xargs grep -l "enum OrderStatus\|type OrderStatus" | head -10
echo ""
echo "=== Search in found files ==="
rg "enum OrderStatus|type OrderStatus" --type ts -A 15 | head -80
echo ""
echo "=== Check git history for the enum to see if CREATION_MINED value changed ==="
git log --oneline -n 20 -- "apps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.ts" 2>/dev/null || echo "Git not available"Repository: cowprotocol/cowswap
Length of output: 5756
Enum value mismatch warrants verification against backend contracts.
The enum key CREATION_MINED has a string value of 'CREATED' instead of 'CREATION_MINED'. While this mismatch is preserved from existing code and doesn't break the current implementation (the code uses the enum key, not the string value), confirm this value aligns with any backend/API contracts before proceeding.
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/modules/ethFlow/pure/EthFlowStepper/types.ts lines
1-6, the enum member CREATION_MINED currently maps to the string 'CREATED' which
mismatches the key; update the enum so the CREATION_MINED key maps to the string
'CREATION_MINED' (or change to whatever exact token the backend/API expects),
confirm the canonical value with the backend/contracts, and run related
unit/integration tests to ensure nothing breaks from the change.
| { | ||
| content: ( | ||
| <span> | ||
| <Trans>Place orders for higher than available balance!</Trans> | ||
| </span> | ||
| ), | ||
| }, |
There was a problem hiding this comment.
Use msg instead of Trans for Fast Refresh compatibility.
Item 6 uses a Trans component wrapped in JSX, while items 1–5 use the msg macro. This inconsistency introduces a React element into a constants module, which may interfere with Fast Refresh—the primary goal of this PR.
Since the text "Place orders for higher than available balance!" contains no interpolation or special formatting, it should use msg like the other items.
🔎 Proposed fix
- {
- content: (
- <span>
- <Trans>Place orders for higher than available balance!</Trans>
- </span>
- ),
- },
+ { content: msg`Place orders for higher than available balance!` },Remove the unused Trans import:
-import { msg } from '@lingui/core/macro'
-import { Trans } from '@lingui/react/macro'
+import { msg } from '@lingui/core/macro'Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/limitOrders/containers/LimitOrdersWidget/constants.tsx
around lines 12 to 18, the item uses a JSX <Trans> element while other items use
the msg macro; replace the Trans usage with msg("Place orders for higher than
available balance!") so the constant is a plain string message (no React
element) and remove the now-unused Trans import to maintain Fast Refresh
compatibility.
| /** | ||
| * Authenticate once on start | ||
| */ | ||
| useEffect(() => { | ||
| authenticate() | ||
| }, [authenticate]) |
There was a problem hiding this comment.
Duplicate authentication call on mount.
The useTgAuthorization hook already calls authenticate() internally on mount (see useTgAuthorization.ts lines 103-106). This additional useEffect creates a redundant authentication request.
🔎 Proposed fix - remove duplicate effect
- /**
- * Authenticate once on start
- */
- useEffect(() => {
- authenticate()
- }, [authenticate])
-
const isLoading = isTelegramScriptLoading || !isAuthChecked || isCmsCallInProgress || isLoginInProgressCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.ts
around lines 56-61, there is a duplicate useEffect that calls authenticate() on
mount even though useTgAuthorization already authenticates on mount; remove this
redundant useEffect block (the entire useEffect that calls authenticate()),
leaving any other logic intact and ensuring no other references depend on that
effect.
apps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsx
Outdated
Show resolved
Hide resolved
| import Delegate from '../Delegate' | ||
| import { CardsLoader } from '../styled' | ||
|
|
||
| export { PageViewKeys } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for imports of PageViewKeys from TokensOverview
echo "=== Searching for imports of PageViewKeys from TokensOverview ==="
rg -n "import.*PageViewKeys.*from.*['\"].*TokensOverview" --type=ts --type=tsx
echo -e "\n=== Checking the TokensOverview.tsx file for PageViewKeys import/export ==="
# Read the first 80 lines to see imports and the export statement
head -80 "apps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsx" | cat -n
echo -e "\n=== Searching for any imports of PageViewKeys in the codebase ==="
rg -n "import.*PageViewKeys" --type=ts --type=tsx | head -20Repository: cowprotocol/cowswap
Length of output: 3133
Remove the unnecessary re-export of PageViewKeys.
No other modules import PageViewKeys from TokensOverview. Since it's only used internally within the file (imported from ./types and used in the PageView object definition), the re-export at line 55 is unnecessary and clutters the module interface.
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/pages/Account/Tokens/TokensOverview.tsx around line
55, there is an unnecessary re-export "export { PageViewKeys }"; remove that
export statement so the module no longer exposes PageViewKeys (it remains
imported from ./types and used locally in the PageView object). After removal,
run a quick project-wide search to confirm no other files depend on importing
PageViewKeys from this file; if any do, update those imports to import from the
original ./types location instead.
| export const [mockedConnector, mockedConnectorHooks] = initializeConnector<MockedConnector>( | ||
| (actions) => new MockedConnector(actions), | ||
| ) | ||
| import { LanguageProvider } from './i18n' |
There was a problem hiding this comment.
Fix import/export name mismatch.
This line imports LanguageProvider from './i18n', but the i18n.tsx file exports the component as Provider, not LanguageProvider. This will cause a module resolution error at runtime.
🔎 Proposed fix
-import { LanguageProvider } from './i18n'
+import { Provider as LanguageProvider } from './i18n'Alternatively, if you prefer to update the export in i18n.tsx:
In apps/cowswap-frontend/src/lib/i18n.tsx:
-export function Provider({ locale, onActivate, children }: ProviderProps): ReactNode {
+export function Provider({ locale, onActivate, children }: ProviderProps): ReactNode {
// ... implementation
}
+
+export { Provider as LanguageProvider }📝 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.
| import { LanguageProvider } from './i18n' | |
| import { Provider as LanguageProvider } from './i18n' |
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/test-utils.tsx around line 58, the file imports
LanguageProvider from './i18n' but the actual export in i18n.tsx is named
Provider, causing an import/export mismatch; fix by either updating this import
to import { Provider as LanguageProvider } from './i18n' or change the export
name in apps/cowswap-frontend/src/lib/i18n.tsx to export const LanguageProvider
= Provider (or export default LanguageProvider) so the imported name matches the
exported symbol.
| // TODO: Replace any with proper type definitions | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const colour = (theme as any)[color] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Address the type assertion TODO.
The use of theme as any bypasses TypeScript's type checking for the theme object. This should be addressed to ensure type safety.
Do you want me to help generate a properly typed solution, or would you like me to open an issue to track this technical debt?
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/theme/components.tsx around lines 12–14, the code
uses "theme as any" which disables type checking; replace this by importing or
declaring the proper Theme type (e.g., DefaultTheme from styled-components or
the app's exported Theme interface), then type the theme parameter/variable
appropriately and index it using a safe key type (use keyof Theme or
Theme['colors'] if colors are nested) — e.g., import the Theme type from your
theme file, change the theme annotation to that type, and cast the color index
to keyof Theme (or Theme['colors']) so TypeScript can validate the property
access instead of using any.
| Main(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, | ||
| Link(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, | ||
| Label(props: TextProps) { | ||
| return <TextWrapper fontWeight={600} {...props} /> | ||
| }, | ||
| Small(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} fontSize={11} {...props} /> | ||
| }, | ||
| Black(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, | ||
| Blue(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, | ||
| Error({ ...props }: { error: boolean } & TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, | ||
| Body(props: TextProps) { | ||
| return <TextWrapper fontWeight={400} fontSize={16} {...props} /> | ||
| }, | ||
| MediumHeader(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} fontSize={20} {...props} /> | ||
| }, | ||
| LargeHeader(props: TextProps) { | ||
| return <TextWrapper fontWeight={600} fontSize={24} {...props} /> | ||
| }, | ||
| DarkGray(props: TextProps) { | ||
| return <TextWrapper fontWeight={500} {...props} /> | ||
| }, |
There was a problem hiding this comment.
Significant code duplication in ThemedText presets.
Several ThemedText presets have identical implementations:
Main,Link,Black,Blue, andDarkGrayall render with onlyfontWeight={500}and no other distinguishing properties
This duplication suggests either:
- These presets serve different semantic purposes (and should apply different colors via the
colorprop at call sites), or - The implementations are incomplete and need distinct styling
Additionally, ThemedText.Error (Line 45) destructures an error boolean prop but never uses it, which is misleading.
🔎 Suggested approach
If these presets are semantically distinct (e.g., Link for links, Error for error text), consider either:
Option 1: Set default colors in each preset:
Link(props: TextProps) {
- return <TextWrapper fontWeight={500} {...props} />
+ return <TextWrapper fontWeight={500} color="blue1" {...props} />
},
Error({ error, ...props }: { error: boolean } & TextProps) {
- return <TextWrapper fontWeight={500} {...props} />
+ return <TextWrapper fontWeight={500} color={error ? 'red1' : 'text1'} {...props} />
},Option 2: If they're truly identical, consolidate them or document why separate names are needed for semantic clarity.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/theme/components.tsx around lines 27-59, several
ThemedText presets (Main, Link, Black, Blue, DarkGray) are duplicate
implementations and ThemedText.Error destructures an unused error prop; either
assign distinct default styling (e.g., set specific color props and any other
differing fontSize/weight) for each semantic preset or consolidate duplicate
presets into a single alias export with a clear comment, and for Error either
remove the unused destructured error prop or use it to conditionally set a
color/variant (and adjust the prop type to mark error as optional if needed);
update/remove redundant functions accordingly and keep the public API
consistent.
if I need to do it, then it would be great to add a description what and how to test ;) |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
apps/cowswap-frontend/src/theme/components.tsx (2)
46-57: Type assertion issue already flagged.The
theme as anytype assertion on line 50 bypasses TypeScript type checking. This has been previously identified and documented in past review comments.
62-96: Code duplication and unused prop issues already flagged.Multiple ThemedText presets have identical implementations (Main, Link, Black, Blue, DarkGray), and the Error preset has an unused
errorboolean prop. These issues have been previously identified and documented in past review comments.apps/cowswap-frontend/src/common/hooks/useBlockNumber/context.ts (1)
1-12: LGTM! Clean context module that resolves the previous type issue.The context module correctly separates the block number context definition and achieves the PR's fast refresh objective. The previous reviewer's concern about attaching
MISSING_PROVIDERas a property toBlockNumberContexthas been resolved—that problematic line is no longer present in the current implementation.
Note: The past review comment referenced line 14 with
BlockNumberContext.MISSING_PROVIDER = MISSING_PROVIDER, but this line does not exist in the current code, indicating the issue has already been addressed.
🧹 Nitpick comments (3)
apps/cowswap-frontend/src/theme/components.tsx (1)
23-44: Remove or implement unused props.The
bgandisCopiedprops are defined in the type signature but never used in the component's styling logic. This can be misleading to consumers.🔎 Remove unused props
-export const LinkStyledButton = styled.button<{ disabled?: boolean; bg?: boolean; isCopied?: boolean; color?: string }>` +export const LinkStyledButton = styled.button<{ disabled?: boolean; color?: string }>`Alternatively, if these props are intended for future use or external styling, document their purpose or implement their behavior.
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.ts (1)
10-27: Consider migrating hardcoded colors to the UI design system.The map uses a mix of UI tokens (e.g.,
var(${UI.COLOR_BLUE_300_PRIMARY})) and hardcoded hex colors (#FFDB9C,#f0dede). For better theme consistency and maintainability, consider adding the hardcoded colors to the UI design system as semantic color tokens.apps/cowswap-frontend/src/locales/en-US.po (1)
3352-3353: Obsolete BlockNumber hook string is safe; consider pruning laterThe
BlockNumber hooks must be wrapped in aentry is now marked obsolete (#~), so it no longer affects the UI. That’s correct if the underlying message was removed or replaced. If you want to keep locale files lean, you can drop this obsolete entry on a future extraction run, but it’s not urgent.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/cowswap-frontend/src/common/hooks/useBlockNumber/context.tsapps/cowswap-frontend/src/common/hooks/useBlockNumber/useBlockNumber.tsapps/cowswap-frontend/src/locales/en-US.poapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsxapps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsxapps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.tsapps/cowswap-frontend/src/theme/components.tsxapps/cowswap-frontend/src/theme/types.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/cowswap-frontend/src/theme/types.ts
- apps/cowswap-frontend/src/modules/notifications/containers/NotificationSettings.tsx
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-10-10T20:28:16.565Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6347
File: apps/cowswap-frontend/src/modules/trade/pure/TradeConfirmation/index.tsx:49-49
Timestamp: 2025-10-10T20:28:16.565Z
Learning: In apps/cowswap-frontend/src/modules/trade, TradeConfirmation follows a two-layer architecture: TradeConfirmationView (pure/stateless) in pure/TradeConfirmation/index.tsx renders the UI, while TradeConfirmation (container) in containers/TradeConfirmation/index.tsx wraps it to freeze props during pending trades (via useStableTradeConfirmationProps), wire in signing state (useSigningStep), and inject trade confirmation state (useTradeConfirmState). Consuming modules should import the container TradeConfirmation from 'modules/trade' to preserve this stateful behavior.
<!-- [add_learning]
When reviewing component refactoring in apps/cowswap-frontend/src/modules/trade, recognize the pattern where a pure view component (e.g., TradeConfirmationView) is separated from a stateful container (e.g., TradeConfirmation) that wraps it. The container adds runtime state management (prop stabilization, signing state, etc.) while the view remains testable and composable. Do not flag usages that import th...
Applied to files:
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsxapps/cowswap-frontend/src/theme/components.tsxapps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx
📚 Learning: 2025-05-28T16:50:12.273Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 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/cowswap-frontend/src/theme/components.tsx
📚 Learning: 2025-02-20T15:59:33.749Z
Learnt from: shoom3301
Repo: cowprotocol/cowswap PR: 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/locales/en-US.po
📚 Learning: 2025-07-24T16:42:53.154Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 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/locales/en-US.po
📚 Learning: 2025-09-19T11:38:59.206Z
Learnt from: fairlighteth
Repo: cowprotocol/cowswap PR: 6232
File: apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx:199-200
Timestamp: 2025-09-19T11:38:59.206Z
Learning: The makeBuildClickEvent function in apps/cowswap-frontend/src/modules/tokensList/pure/ChainsSelector/index.tsx takes five parameters: defaultChainId, contextLabel, mode, isSwapMode, and chainsCount. The chainsCount parameter is used to determine the CrossChain flag in analytics events.
Applied to files:
apps/cowswap-frontend/src/locales/en-US.po
📚 Learning: 2025-07-24T10:00:45.353Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 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/locales/en-US.po
📚 Learning: 2025-07-24T16:43:47.639Z
Learnt from: cowdan
Repo: cowprotocol/cowswap PR: 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/locales/en-US.po
📚 Learning: 2025-10-13T19:41:31.440Z
Learnt from: limitofzero
Repo: cowprotocol/cowswap PR: 6351
File: apps/cowswap-frontend/src/modules/erc20Approve/containers/TradeApproveModal/useTradeApproveCallback.ts:87-121
Timestamp: 2025-10-13T19:41:31.440Z
Learning: In apps/cowswap-frontend/src/modules/erc20Approve, useApproveCallback returns Promise<TransactionResponse | undefined> and is distinct from useApproveCurrency, which can return Promise<TransactionReceipt | SafeMultisigTransactionResponse>. When reviewing approval flows, verify which hook is actually being used before flagging Safe wallet concerns.
Applied to files:
apps/cowswap-frontend/src/locales/en-US.po
🧬 Code graph analysis (3)
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.ts (2)
apps/cowswap-frontend/src/modules/orderProgressBar/types.ts (1)
OrderProgressBarProps(36-51)apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsx (1)
PROCESS_IMAGE_WRAPPER_BG_COLOR(12-12)
apps/cowswap-frontend/src/theme/components.tsx (3)
libs/types/src/common.ts (1)
Command(3-3)apps/cowswap-frontend/src/theme/types.ts (1)
TextProps(5-5)apps/cowswap-frontend/src/theme/consts.tsx (1)
TextProps(3-3)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (1)
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram/useConnectTelegram.ts (1)
ConnectTelegramController(14-23)
⏰ 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). (2)
- GitHub Check: Setup
- GitHub Check: Cypress
🔇 Additional comments (8)
apps/cowswap-frontend/src/common/hooks/useBlockNumber/useBlockNumber.ts (1)
1-13: LGTM! Hook implementation is correct and achieves fast refresh compatibility.The hook correctly:
- Checks for the missing provider sentinel and throws a helpful error
- Leverages TypeScript's type narrowing after the equality check (line 8) to safely access
blockNumber.valueon line 12- Exports only a hook function, which is compatible with React Fast Refresh rules
This refactoring successfully separates the hook into its own module, supporting the PR's objective to fix HMR full-reload issues.
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsx (2)
3-7: Clean constant extraction.The import of constants from a dedicated module aligns with the Fast Refresh compatibility goal.
19-30: Component logic is correct.The component properly uses the imported constants while maintaining the same behavior as before.
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper/constants.ts (2)
5-8: Well-typed constant maps.The type definitions using
Record<NonNullable<StepName>, string | undefined>ensure compile-time completeness—all step names must have an entry in each map.
29-65: Padding and gap maps are correct.The maps provide consistent styling configuration for all step names, with appropriate use of
undefinedfor steps that don't require padding or gap.apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx (1)
5-6: LGTM: Clean container pattern with properly extracted hook.The hook extraction to a separate module and the container/controller pattern implementation are well-structured. The component correctly receives the controller and passes its properties to the child
TelegramConnectionStatuscomponent.Also applies to: 14-34
apps/cowswap-frontend/src/locales/en-US.po (2)
274-276: StopStatus i18n contexts now reference constants.tsx (LGTM)The source references for bridge stop-status strings now point to
apps/cowswap-frontend/src/modules/bridge/pure/StopStatus/constants.tsx, matching the refactor that centralizes these constants. Msgids/msgstrs are unchanged, so runtime text and translations remain intact while tooling navigation and Fast Refresh compatibility improve.Also applies to: 1415-1417, 1945-1947, 2505-2507, 2624-2626, 3678-3680, 3926-3928, 4887-4889, 4991-4993
294-296: LimitOrdersWidget marketing strings moved to constants.tsx (LGTM)These limit-order promo/UX strings are now sourced from
LimitOrdersWidget/constants.tsx(and additionally referenced fromAdvancedOrdersWidgetfor the shared “Receive surplus of your order” copy). Text content is unchanged; only location metadata moved, which is consistent with extracting constants out of container components for React Refresh friendliness.Also applies to: 456-458, 1014-1016, 1131-1133, 1207-1209, 1363-1365, 3737-3739, 4428-4430, 4519-4521, 4576-4578
| export type { ConnectTelegramController } | ||
| export { useConnectTelegram } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx | head -30Repository: cowprotocol/cowswap
Length of output: 1253
🏁 Script executed:
rg -n --type=ts --type=tsx -C3 "from ['\"].*notifications/containers/ConnectTelegram['\"]" -g '!**/ConnectTelegram.tsx' -g '!**/useConnectTelegram.ts'Repository: cowprotocol/cowswap
Length of output: 90
🏁 Script executed:
rg "from.*notifications/containers/ConnectTelegram" --type ts --type typescriptRepository: cowprotocol/cowswap
Length of output: 97
🏁 Script executed:
rg "from.*notifications/containers/ConnectTelegram" -g "*.ts" -g "*.tsx"Repository: cowprotocol/cowswap
Length of output: 45
🏁 Script executed:
rg "useConnectTelegram|ConnectTelegramController" -g "*.ts" -g "*.tsx" | head -20Repository: cowprotocol/cowswap
Length of output: 1186
Remove hook and type re-exports to restore Fast Refresh compatibility.
This file exports both a React component (ConnectTelegram) and non-component exports (useConnectTelegram hook and ConnectTelegramController type), which violates React Fast Refresh requirements. When a module exports non-React components, Fast Refresh will re-run both that module and the other modules importing it, defeating the purpose of this refactor.
Update NotificationSettings.tsx to import the hook directly from ./ConnectTelegram/useConnectTelegram instead of from this component file, then remove the re-exports on lines 9-10.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/notifications/containers/ConnectTelegram.tsx
around lines 9-10, this file currently re-exports the useConnectTelegram hook
and ConnectTelegramController type which breaks React Fast Refresh; remove the
two export lines (the re-exports of useConnectTelegram and
ConnectTelegramController) from this file and instead update
NotificationSettings.tsx to import the hook directly from
./ConnectTelegram/useConnectTelegram (and import the type from its original
declaration if needed) so only the React component remains exported from
ConnectTelegram.tsx.
| import { ProgressImageWrapper as ProgressImageWrapperEl } from '../sharedStyled' | ||
| import { OrderProgressBarProps } from '../types' | ||
|
|
||
| export { PROCESS_IMAGE_WRAPPER_BG_COLOR } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all imports of PROCESS_IMAGE_WRAPPER_BG_COLOR from ProgressImageWrapper.tsx
echo "=== Searching for imports from ProgressImageWrapper ==="
rg -n --type=ts --type=tsx "from ['\"].*ProgressImageWrapper"
echo ""
echo "=== Checking if PROCESS_IMAGE_WRAPPER_BG_COLOR is imported from ProgressImageWrapper anywhere ==="
rg -n --type=ts --type=tsx "from.*ProgressImageWrapper.*PROCESS_IMAGE_WRAPPER_BG_COLOR|import.*PROCESS_IMAGE_WRAPPER_BG_COLOR.*from.*ProgressImageWrapper"
echo ""
echo "=== Checking the ProgressImageWrapper.tsx file ==="
find . -name "ProgressImageWrapper.tsx" -type f | head -5Repository: cowprotocol/cowswap
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Read the ProgressImageWrapper.tsx file to see the export
echo "=== Content of ProgressImageWrapper.tsx ==="
cat ./apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsx
echo ""
echo "=== All references to PROCESS_IMAGE_WRAPPER_BG_COLOR in codebase ==="
rg -n "PROCESS_IMAGE_WRAPPER_BG_COLOR"
echo ""
echo "=== Check for imports in TopSections.tsx ==="
find . -name "TopSections.tsx" -o -name "TopSections.ts" | xargs cat 2>/dev/null | head -50Repository: cowprotocol/cowswap
Length of output: 3957
Remove the re-export of PROCESS_IMAGE_WRAPPER_BG_COLOR from ProgressImageWrapper.tsx.
The constant is not imported by any consumers from this file. TopSections.tsx correctly imports directly from the constants module (./ProgressImageWrapper/constants), and the re-export on line 12 is unused. Removing it eliminates a potential source of Fast Refresh issues while maintaining the correct import path for consumers.
🤖 Prompt for AI Agents
In
apps/cowswap-frontend/src/modules/orderProgressBar/pure/ProgressImageWrapper.tsx
around line 12, remove the unused re-export "PROCESS_IMAGE_WRAPPER_BG_COLOR"
from this file; it's not consumed here and TopSections.tsx already imports it
from ./ProgressImageWrapper/constants, so delete the export statement to avoid
Fast Refresh issues and leave the original constants module as the single source
of truth.
| // We start the updater with a delay | ||
| const LP_UPDATER_START_DELAY = ms`3s` | ||
|
|
||
| export const areLpBalancesLoadedAtom = atom(false) |
There was a problem hiding this comment.
Fast refresh only works when a file only exports components. Use a new file to share constants or functions between components react-refresh/only-export-components
Lesson learned
shoom3301
left a comment
shoom3301
left a comment
There was a problem hiding this comment.
This is amazing!
Thanks a lot!
|
Postponing this PR a bit, I asked the AI to fix these exports and it introduced some errors, I will manually review or I will manually split the exports myself. |
kernelwhisperer
force-pushed
the
fix/fast-refresh
branch
from
94ecee0 to
58d30ac
Compare
Summary
Fixes #6738 (fast refresh triggering a full reload)
Adds a new eslint rule to warn about incompatible exports + fixing them with AI:
useIsNoImpactWarningAccepted.tsconstants.tsxorderFillabilityUtils.tscommon/constants/dateFormats.tslpBalancesState.ts(atom separated)types.ts(enum separated)utils.ts(determinePillColour function)utils.ts(getBorderWidth function)constants.tsx(all status constants)types.ts(enums and interfaces)useConnectTelegram.ts(hook separated)constants.ts(style maps)types.ts(enum separated)types.ts(PageViewKeys enum)test-utils/mocks.tsx(mock connectors)zIndex.tsandtypes.ts(enum and types separated)Testing
Not sure if it's worth doing a manual review or manual testing. Thoughts?
Summary by CodeRabbit
Release Notes
New Features
Improvements
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.