Skip to content

Conversation

@QuentinGibson
Copy link
Contributor

Fix long SFAPI language codes

WHY are these changes introduced?

Fixes #1059
There is a bug with languageCodes like PT_PT, PT_BR

WHAT is this pull request doing?

Checking if the languageCode is long enough to be the local in useMoney

HOW to test your changes?

All tests passed. I ran npm run test

Checklist

  • I've read the Contributing Guidelines
  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've added a changeset if this PR contains user-facing or noteworthy changes
  • I've added tests to cover my changes
  • I've added or updated the documentation

@QuentinGibson QuentinGibson marked this pull request as ready for review July 17, 2023 23:36
@QuentinGibson QuentinGibson force-pushed the fix-long-language-codes-use-money branch from 857a4d5 to efdedde Compare July 17, 2023 23:45
@QuentinGibson
Copy link
Contributor Author

@wizardlyhel This checks for the long languagecodes and formats it to work with the Intl.NumberFormat.

Copy link

@wizardlyhel wizardlyhel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix!

Would you run npm run changeset, select @shopify/hydrogen-react, and select patch change.

Would you add this test as well in useMoney.test.tsx. Watch out for the invisible character. If it doesn't copy well, inside the packages/hydrogen-react

  1. run npm test:watch
  2. replace expect(result.current).toEqual(... with expect(result.current).toMatchInlineSnapshot();
  3. save to get a copy of the invisible space character
import {describe, expect, it} from 'vitest';
import {renderHook} from '@testing-library/react';
import {ShopifyProvider, ShopifyProviderProps} from './ShopifyProvider.js';
import {useMoney} from './useMoney.js';

...

  it(`does not fail when language ISO code is more than 2 characters`, () => {
    const SHOPIFY_CONFIG: ShopifyProviderProps = {
      storeDomain: 'https://notashop.myshopify.com',
      storefrontToken: 'abc123',
      storefrontApiVersion: '2023-04',
      countryIsoCode: 'BR',
      languageIsoCode: 'PT_PT',
    };

    const {result} = renderHook(() => useMoney({
      amount: '19.00',
      currencyCode: 'USD',
    }), {
      wrapper: ({children}) => (
        <ShopifyProvider
          {...SHOPIFY_CONFIG}
          storeDomain="https://notashop.myshopify.com"
        >
          {children}
        </ShopifyProvider>
      ),
    });

    expect(result.current).toEqual({
      amount: '19,00 ',
      currencyCode: 'USD',
      currencyName: 'dólares dos Estados Unidos',
      currencyNarrowSymbol: '$',
      currencySymbol: 'US$',
      localizedString: '19,00 US$',
      original: {
        amount: '19.00',
        currencyCode: 'USD',
      },
      parts: [
        {
          type: 'integer',
          value: '19',
        },
        {
          type: 'decimal',
          value: ',',
        },
        {
          type: 'fraction',
          value: '00',
        },
        {
          type: 'literal',
          value: ' ',
        },
        {
          type: 'currency',
          value: 'US$',
        },
      ],
      withoutTrailingZeros: '19 US$',
      withoutTrailingZerosAndCurrency: '19',
    });
  });

@wizardlyhel
Copy link

We need to merge this fix - closed with your fix migrated to this PR #1132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] useMoney Hook Error: Incorrect locale provided (hydrogen-react package)

2 participants