fix: currency interface #208
Conversation
WalkthroughThis pull request modifies TypeScript declaration files for two components: Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/create-invoice-form/src/lib/react/CreateInvoiceForm.d.ts (1)
28-28: Consider enhancing the example documentationWhile the simplified example is correct, it would be more helpful to show the expected structure of the currencies array for better developer experience.
Consider updating the example to show the structure:
-* currencies={currencies} +* currencies={[ +* { +* symbol: "DAI", +* address: "0x...", +* network: "mainnet", +* decimals: 18, +* type: "ERC20" +* } +* ]}packages/invoice-dashboard/src/lib/react/InvoiceDashboard.d.ts (1)
Line range hint
14-31: Document the breaking change in JSDoc.While the JSDoc is comprehensive, it should document the type requirements for the
currenciesprop to help consumers migrate correctly.Add type details to the
@paramsection:* @param {InvoiceDashboardProps} props - The component props +* @param {InvoiceDashboardProps['currencies']} props.currencies - Array of currency inputs following the CurrencyInput type structure * @returns {JSX.Element}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
packages/create-invoice-form/src/lib/react/CreateInvoiceForm.d.ts(2 hunks)packages/invoice-dashboard/src/lib/react/InvoiceDashboard.d.ts(1 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/invoice-dashboard/src/lib/react/InvoiceDashboard.d.ts (2)
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: packages/invoice-dashboard/src/lib/react/InvoiceDashboard.d.ts:11-11
Timestamp: 2024-11-12T14:52:33.204Z
Learning: When reviewing PRs, understand that updating the payment-widget package is considered out of scope unless specified in the PR objectives, even if inconsistencies like currency type usage are detected.
Learnt from: MantisClone
PR: RequestNetwork/web-components#141
File: packages/invoice-dashboard/src/utils/generateInvoice.ts:0-0
Timestamp: 2024-11-12T14:52:33.204Z
Learning: In `packages/invoice-dashboard/src/utils/generateInvoice.ts`, the `paymentCurrencies` parameter in the `exportToPDF` function is intended to have the type `any[]`.
🔇 Additional comments (3)
packages/create-invoice-form/src/lib/react/CreateInvoiceForm.d.ts (2)
4-5: LGTM! Import changes align with the new type usage.
The switch to importing CurrencyTypes from @requestnetwork/types is appropriate as it's the canonical source for currency-related type definitions.
11-11: Verify backward compatibility of CurrencyInput type
The change to CurrencyTypes.CurrencyInput[] aligns with PR #207, but let's verify that existing consumers won't be affected.
✅ Verification successful
No breaking changes in CurrencyInput type usage
Based on the evidence gathered, the CurrencyTypes.CurrencyInput[] type is consistently used across multiple components including InvoiceDashboard and CreateInvoiceForm. The type is used in the same way in both the previous and current versions of the interface, and its usage patterns in the codebase (shared utils, dashboard, and form components) show it's a stable type definition. The change maintains backward compatibility as there's no modification to the underlying type structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing usage patterns of the currencies prop
# to ensure compatibility with the new type
# Look for CreateInvoiceForm usage with currencies prop
rg -A 5 "CreateInvoiceForm.*currencies="
# Look for type definitions of CurrencyInput to verify structure
ast-grep --pattern 'type CurrencyInput = $_'
Length of output: 89
Script:
#!/bin/bash
# Let's try a broader search to understand the currency types and usage
# Search for currency type definitions
rg -l "CurrencyTypes"
# Search for CreateInvoiceForm usage
rg -l "CreateInvoiceForm"
# Once we have the files, let's examine the specific one with type definitions
fd "Currency.*\.ts" --type f
Length of output: 1234
Script:
#!/bin/bash
# Let's examine the currency type definitions and usage
# Check the currency type definition in shared utils
cat shared/utils/getCurrency.ts
# Look at the implementation of CreateInvoiceForm to see how currencies are used
cat packages/create-invoice-form/src/lib/react/CreateInvoiceForm.jsx
# Check the previous type definition in git history
git show HEAD~1:packages/create-invoice-form/src/lib/react/CreateInvoiceForm.d.ts
Length of output: 2489
Script:
#!/bin/bash
# Let's check the actual CurrencyTypes definition and any usage patterns
# Look for the definition of CurrencyInput type
rg -A 10 "CurrencyInput"
# Check the web component implementation for currency handling
cat packages/create-invoice-form/src/lib/web-component.ts
Length of output: 6339
packages/invoice-dashboard/src/lib/react/InvoiceDashboard.d.ts (1)
11-11: Verify the currency type change impact.
The change from CurrencyDefinition[] to CurrencyInput[] aligns with the PR objectives. However, this is a breaking change that requires verification.
Let's verify the currency type usage across the codebase:
✅ Verification successful
Let me try a different approach with the correct file type flags for ripgrep and a more specific AST pattern.
Let me try one final approach using simpler ripgrep patterns without file type flags and more focused ast-grep patterns:
Breaking change in currency type is consistent with codebase
The type change from CurrencyDefinition[] to CurrencyInput[] in InvoiceDashboard.d.ts is correct and consistent with the codebase for the following reasons:
-
The
currenciesprop of typeCurrencyInput[]is already being used in other components:shared/utils/initCurrencyManager.ts:customCurrencies: CurrencyTypes.CurrencyInput[]packages/invoice-dashboard/src/lib/view-requests.svelte:currencies: CurrencyTypes.CurrencyInput[]packages/create-invoice-form/src/lib/create-invoice-form.svelte:currencies: CurrencyTypes.CurrencyInput[]
-
The change aligns with the internal implementation where
CurrencyInputis used for initial currency configuration, whileCurrencyDefinitionis used for processed/resolved currency objects in operations like conversions and payments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify currency type usage and potential breaking changes
# Check for CurrencyDefinition usage
echo "Checking for CurrencyDefinition usage..."
rg -t typescript "CurrencyDefinition\[\]"
# Check for CurrencyInput usage
echo "Checking for CurrencyInput usage..."
rg -t typescript "CurrencyInput\[\]"
# Check for direct currencies prop usage
echo "Checking for currencies prop usage..."
ast-grep --pattern 'currencies={$_}'
Length of output: 448
Script:
#!/bin/bash
# Description: Verify currency type usage and potential breaking changes
# Check for CurrencyDefinition usage
echo "Checking for CurrencyDefinition usage..."
rg "CurrencyDefinition\[\]" --type ts --type tsx
# Check for CurrencyInput usage
echo "Checking for CurrencyInput usage..."
rg "CurrencyInput\[\]" --type ts --type tsx
# Check for direct currencies prop usage in TSX/JSX files
echo "Checking for currencies prop usage..."
ast-grep --pattern 'InvoiceDashboard $$$props={$$$} currencies={$_}'
# Check currency type imports
echo "Checking currency type imports..."
rg "import.*CurrencyTypes" --type ts --type tsx
Length of output: 635
Script:
#!/bin/bash
# Search for any currency type references
echo "Checking currency type references..."
rg "Currency(Definition|Input)"
# Look for InvoiceDashboard component usage
echo "Checking InvoiceDashboard usage..."
rg "InvoiceDashboard"
# Check for currency-related imports
echo "Checking currency imports..."
rg "import.*Currency"
# Look for InvoiceDashboard JSX/TSX usage with currencies prop
echo "Checking InvoiceDashboard currencies prop usage..."
ast-grep --pattern '<InvoiceDashboard $$$props currencies={$_}'
Length of output: 10044
Towards #76
Fixup #207
Problem
The previous PR #207 correctly made the
currencyinput parameter typeCurrencyInput[]but neglected to fix the types.d.tsfiles.Solution
.d.tsfiles to useCurrencyInput[]Summary by CodeRabbit
CreateInvoiceFormandInvoiceDashboardcomponents to support a simplified currency input structure.currenciesprop structure in the documentation.