chore: allow to set orderbook urls using local storage #6003
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes introduce support for loading order book URLs from both environment variables and browser localStorage in two frontend modules. The README is updated to fix a JSON syntax error in an environment variable example. No exported or public entity declarations are altered. Changes
Estimated code review effort2 (~15 minutes) Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
yarn install v1.22.22 ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/cowswap-frontend/src/cowSdk.ts (1)
16-16: Consider reducing console logging verbosity in production.The console logging is helpful for development but may be too verbose for production environments. Consider using a debug flag or logging level.
Example approach:
-console.log('Order Book URLs:', prodBaseUrls, !!envBaseUrls, !!localStorageBaseUrls) +if (process.env.NODE_ENV === 'development') { + console.log('Order Book URLs:', prodBaseUrls, !!envBaseUrls, !!localStorageBaseUrls) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(1 hunks)apps/cowswap-frontend/src/cowSdk.ts(1 hunks)apps/explorer/src/cowSdk.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
apps/cowswap-frontend/src/cowSdk.ts (1)
Learnt from: shoom3301
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.
🔇 Additional comments (2)
README.md (1)
201-201: LGTM! Documentation bug fix.The missing closing curly brace in the JSON example has been corrected, ensuring the environment variable example is syntactically valid and consistent with the parsing logic in the codebase.
apps/cowswap-frontend/src/cowSdk.ts (1)
7-10: Excellent developer guidance!The comments providing clear instructions for manually setting and clearing localStorage URLs are very helpful for developers and testers.
| const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | ||
|
|
||
| // To manually set the order book URLs in localStorage, you can use the following command in the browser console: | ||
| // localStorage.setItem('orderBookUrls', JSON.stringify({ "1":"https://YOUR_HOST", "100":"https://YOUR_HOST" })) | ||
| // To clear it, simply run: | ||
| // localStorage.removeItem('orderBookUrls') | ||
| const localStorageBaseUrls = | ||
| localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}') | ||
|
|
||
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined |
There was a problem hiding this comment.
Add error handling for JSON parsing operations.
Similar to the explorer implementation, this code lacks error handling for malformed JSON, which could cause application crashes.
Apply this diff to add proper error handling:
-const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS)
+const envBaseUrls = (() => {
+ try {
+ return process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS)
+ } catch (error) {
+ console.warn('Failed to parse REACT_APP_ORDER_BOOK_URLS:', error)
+ return undefined
+ }
+})()
-const localStorageBaseUrls =
- localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}')
+const localStorageBaseUrls = (() => {
+ try {
+ const stored = localStorage.getItem('orderBookUrls')
+ return stored && JSON.parse(stored)
+ } catch (error) {
+ console.warn('Failed to parse orderBookUrls from localStorage:', error)
+ return undefined
+ }
+})()📝 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.
| const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | |
| // To manually set the order book URLs in localStorage, you can use the following command in the browser console: | |
| // localStorage.setItem('orderBookUrls', JSON.stringify({ "1":"https://YOUR_HOST", "100":"https://YOUR_HOST" })) | |
| // To clear it, simply run: | |
| // localStorage.removeItem('orderBookUrls') | |
| const localStorageBaseUrls = | |
| localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}') | |
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined | |
| const envBaseUrls = (() => { | |
| try { | |
| return process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | |
| } catch (error) { | |
| console.warn('Failed to parse REACT_APP_ORDER_BOOK_URLS:', error) | |
| return undefined | |
| } | |
| })() | |
| // To manually set the order book URLs in localStorage, you can use the following command in the browser console: | |
| // localStorage.setItem('orderBookUrls', JSON.stringify({ "1":"https://YOUR_HOST", "100":"https://YOUR_HOST" })) | |
| // To clear it, simply run: | |
| // localStorage.removeItem('orderBookUrls') | |
| const localStorageBaseUrls = (() => { | |
| try { | |
| const stored = localStorage.getItem('orderBookUrls') | |
| return stored && JSON.parse(stored) | |
| } catch (error) { | |
| console.warn('Failed to parse orderBookUrls from localStorage:', error) | |
| return undefined | |
| } | |
| })() | |
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined |
🤖 Prompt for AI Agents
In apps/cowswap-frontend/src/cowSdk.ts around lines 5 to 14, the JSON.parse
calls for environment and localStorage values lack error handling, risking
crashes on malformed JSON. Wrap each JSON.parse call in try-catch blocks to
safely handle parsing errors, and fallback to undefined or an empty object if
parsing fails, ensuring the app remains stable.
| const prodBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS | ||
| ? JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | ||
| : undefined | ||
| // TODO: why is this duplicated? Can this be shared with the instance from CoW Swap? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address the code duplication concern.
The TODO comment correctly identifies code duplication with the cowswap-frontend implementation. Consider extracting this logic into a shared utility function to maintain consistency and reduce maintenance overhead.
Example approach:
// In a shared utilities file
export function getOrderBookUrls() {
const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS)
const localStorageBaseUrls =
localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}')
return envBaseUrls || localStorageBaseUrls || undefined
}🤖 Prompt for AI Agents
In apps/explorer/src/cowSdk.ts at line 6, there is duplicated logic for
obtaining order book URLs that also exists in the cowswap-frontend codebase. To
fix this, extract this logic into a shared utility function in a common
utilities file that both projects can import. This function should handle
reading from environment variables and local storage as needed, returning the
URLs or undefined. Replace the duplicated code in cowSdk.ts with a call to this
shared utility to ensure consistency and reduce maintenance overhead.
| const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | ||
| const localStorageBaseUrls = | ||
| localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}') | ||
|
|
||
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined |
There was a problem hiding this comment.
Add error handling for JSON parsing operations.
The current implementation lacks error handling for malformed JSON in both environment variables and localStorage. This could cause the application to crash if invalid JSON is provided.
Apply this diff to add proper error handling:
-const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS)
-const localStorageBaseUrls =
- localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}')
+const envBaseUrls = (() => {
+ try {
+ return process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS)
+ } catch (error) {
+ console.warn('Failed to parse REACT_APP_ORDER_BOOK_URLS:', error)
+ return undefined
+ }
+})()
+
+const localStorageBaseUrls = (() => {
+ try {
+ const stored = localStorage.getItem('orderBookUrls')
+ return stored && JSON.parse(stored)
+ } catch (error) {
+ console.warn('Failed to parse orderBookUrls from localStorage:', error)
+ return undefined
+ }
+})()📝 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.
| const envBaseUrls = process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | |
| const localStorageBaseUrls = | |
| localStorage.getItem('orderBookUrls') && JSON.parse(localStorage.getItem('orderBookUrls') || '{}') | |
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined | |
| const envBaseUrls = (() => { | |
| try { | |
| return process.env.REACT_APP_ORDER_BOOK_URLS && JSON.parse(process.env.REACT_APP_ORDER_BOOK_URLS) | |
| } catch (error) { | |
| console.warn('Failed to parse REACT_APP_ORDER_BOOK_URLS:', error) | |
| return undefined | |
| } | |
| })() | |
| const localStorageBaseUrls = (() => { | |
| try { | |
| const stored = localStorage.getItem('orderBookUrls') | |
| return stored && JSON.parse(stored) | |
| } catch (error) { | |
| console.warn('Failed to parse orderBookUrls from localStorage:', error) | |
| return undefined | |
| } | |
| })() | |
| const prodBaseUrls = envBaseUrls || localStorageBaseUrls || undefined |
🤖 Prompt for AI Agents
In apps/explorer/src/cowSdk.ts around lines 8 to 12, the JSON.parse calls for
environment variables and localStorage lack error handling, risking crashes on
malformed JSON. Wrap each JSON.parse operation in try-catch blocks to safely
handle parsing errors. If parsing fails, log the error and fallback to undefined
or an empty object as appropriate to prevent the application from crashing.
|
@MartinquaXD let me know whether this is useful |
Summary
Use localStorage to set a custom backend endpoint
To Test
localStorage.setItem('orderBookUrls', JSON.stringify({ "1":"https://YOUR_HOST", "100":"https://YOUR_HOST" }))localStorage.removeItem('orderBookUrls')Summary by CodeRabbit
New Features
Bug Fixes
Documentation