chore(ramp): upgrade sdk to v2.0.3

[georgeweiler]

Description

This upgrades @consensys/on-ramp-sdk to 2.0.3, which is a major version change that updates the getQuotes and getSellQuotes interfaces.

Related issues

Manual testing steps

1. Go to ramps experience and click "get quotes"
2. confirm that quotes are loading without any issues

Screenshots/Recordings

Before

After

Pre-merge author checklist

• I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
• I've completed the PR template to the best of my ability
• I’ve included tests if applicable
• I’ve documented my code using JSDoc format if applicable
• I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

• I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
• I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

[github-actions]

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

[socket-security]

Updated dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@consensys/[email protected] 🔁 npm/@consensys/[email protected]	None	0	417 kB	consensys-npm

View full report↗︎

[wachunei]

Do this reassignment inside useQuotes, the hook must be reusable and to use it the consumer should not be aware of the data.quotes assignment.

[georgeweiler]

would you suggest that additional metadata properties are extracted, then? This change was made in anticipation of accessing properties like data.sorted. We could do it the way I've done, or something like this:

  const {
    quotes,
    sorted,
    isFetching: isFetchingQuotes,
    error: ErrorFetchingQuotes,
    query: fetchQuotes,
  } = useQuotes(params.amount);

[georgeweiler]

or perhaps we could do something like:

 const {
    data: { quotes,  sorted },
    isFetching: isFetchingQuotes,
    error: ErrorFetchingQuotes,
    query: fetchQuotes,
  } = useQuotes(params.amount);

but we would have to change the interface of data which can be null, currently

[wachunei]

Like your first comment 👌

[wachunei]

useMemo is not needed,

for now the only change needed is

  return {
    data: data?.quotes,

And make sure data.quotes is correctly typed as data was before ((QuoteResponse | QuoteError)[] | (QuoteError | SellQuoteResponse)[] | null)

[wachunei]

I noticed the type changed to (QuoteResponse | QuoteError)[] | (QuoteError | SellQuoteResponse)[] | undefined

@AxelGes what is the reason for that? it should be just nesting the type in a quotes property 🤔

The only change needed in app/components/UI/Ramp/Views/Quotes/Quotes.test.tsx would be

  it('calls setOptions when rendering', async () => {
    mockUseQuotesValues = {
      ...mockUseQuotesInitialValues,
      isFetching: true,
-      data: null,
+      data: undefined,
    };
    render(Quotes);
    expect(mockSetOptions).toBeCalledTimes(1);
  });

[georgeweiler]

I'm a little confused with the suggestion. 😅 Maybe I misunderstood, but didn't we decide in an earlier comment that the useQuotes interface should now spread the data object and become this?

 return {
    quotes,
    // sorted,
    // otherMetadataInTheFuture,
    isFetching,
    error,
    query,
  };

If we want to keep it as data for now, that's fine but I just updated it to be the other way, so looking for some clarification.

If we only updated it to be this:

return {
    data: data?.quotes,
     isFetching,
    error,
    query,
}

1. it's not spreading the data obj like we decided
2. It's risking data to be undefined and not null

[wachunei]

Sorry, later I noticed this step is the backwards compatible one and the other data is not there yet, so to minimize changes we only need these from my last comment.

diff --git a/app/components/UI/Ramp/Views/Quotes/Quotes.test.tsx b/app/components/UI/Ramp/Views/Quotes/Quotes.test.tsx
index 22c84f9f7a..d7ea969ad8 100644
--- a/app/components/UI/Ramp/Views/Quotes/Quotes.test.tsx
+++ b/app/components/UI/Ramp/Views/Quotes/Quotes.test.tsx
@@ -159,7 +159,7 @@ describe('Quotes', () => {
     mockUseQuotesValues = {
       ...mockUseQuotesInitialValues,
       isFetching: true,
-      data: null,
+      data: undefined,
     };
     render(Quotes);
     expect(mockSetOptions).toBeCalledTimes(1);
@@ -200,7 +200,7 @@ describe('Quotes', () => {
     mockUseQuotesValues = {
       ...mockUseQuotesInitialValues,
       isFetching: true,
-      data: null,
+      data: undefined,
     };
     render(Quotes);
     const fetchingQuotesText = screen.getByText('Fetching quotes');
diff --git a/app/components/UI/Ramp/hooks/useQuotes.ts b/app/components/UI/Ramp/hooks/useQuotes.ts
index eea4950794..070ad16d9d 100644
--- a/app/components/UI/Ramp/hooks/useQuotes.ts
+++ b/app/components/UI/Ramp/hooks/useQuotes.ts
@@ -21,7 +21,7 @@ function useQuotes(amount: number | string) {
   );
 
   return {
-    data,
+    data: data?.quotes,
     isFetching,
     error,
     query,

This is enough along with the package.json bump.

It's risking data to be undefined and not null

Yeah not sure about this type change, it is coming from the SDK, however for mobile it would work either way since it checks for null-ish values and not specifically null

[wachunei]

Superseded by #13318