Merge pull request #1487 from Shopify/1473-i18n-merge

[@shopify/react-i18n-universal-provider] Better details merge

packages/react-i18n-universal-provider/CHANGELOG.md

@@ -5,7 +5,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
-<!-- ## [Unreleased] -->
+## [Unreleased]
+
+### Fixed
+
+- Fixed merge of serialized i18n data and override values
 
 ## [1.0.28] - 2019-11-29
 

packages/react-i18n-universal-provider/README.md

@@ -39,13 +39,18 @@ I18nDetails {
 import {I18nUniversalProvider} from '@shopify/react-i18n-universal-provider';
 
 function App({locale}: {locale?: string}) {
-  return <I18nUniversalProvider locale={locale}>{/* rest of the app */}</I18nUniversalProvider>;
+  return (
+    <I18nUniversalProvider locale={locale}>
+      {/* rest of the app */}
+    </I18nUniversalProvider>
+  );
 }
 ```
 
 ### Possible Issues
 
-####  Missing i18n manager error
+#### Missing i18n manager error
+
 ```
 Error: Missing i18n manager. Make sure to use an <I18nContext.Provider /> somewhere in your React tree from the @shopify/react-i18n hook.
 ```
@@ -66,7 +71,7 @@ npx yarn-deduplicate --packages @shopify/react-effect yarn.lock
 ```
 
 ```bash
-$ yarn why @shopify/react-i18n # ensure no duplicate / unmet dependencies 
-yarn list  # ensure no duplicate / unmet dependencies 
+$ yarn why @shopify/react-i18n # ensure no duplicate / unmet dependencies
+yarn list  # ensure no duplicate / unmet dependencies
 yarn install
 ```

packages/react-i18n-universal-provider/src/I18nUniversalProvider.tsx

@@ -3,6 +3,8 @@ import {useLazyRef} from '@shopify/react-hooks';
 import {useSerialized, useHtmlAttributes} from '@shopify/react-html';
 import {I18nContext, I18nDetails, I18nManager} from '@shopify/react-i18n';
 
+import {combinedI18nDetails} from './utilities';
+
 interface Props extends Partial<I18nDetails> {
   children?: React.ReactNode;
 }
@@ -24,11 +26,10 @@ export function I18nUniversalProvider({
 }: Props) {
   const [serialized, Serialize] = useSerialized<Serialized>('i18n');
 
-  const i18nDetails: I18nDetails = {
-    locale: explicitI18nDetails.fallbackLocale || 'en',
-    ...(serialized ? serialized : {}),
-    ...explicitI18nDetails,
-  };
+  const i18nDetails: I18nDetails = combinedI18nDetails(
+    serialized,
+    explicitI18nDetails,
+  );
 
   const manager = useLazyRef(
     () =>

packages/react-i18n-universal-provider/src/test/I18nUniversalProvider.test.tsx

@@ -30,7 +30,6 @@ describe('<I18nUniversalProvider />', () => {
     );
 
     expect(i18n).toProvideReactContext(I18nContext, expect.any(I18nManager));
-
     expect(i18n).toProvideReactContext(
       I18nContext,
       expect.objectContaining({
@@ -81,4 +80,50 @@ describe('<I18nUniversalProvider />', () => {
       }),
     );
   });
+
+  it('overrides serialized data with defined overrides', async () => {
+    const htmlManager = new HtmlManager();
+    const locale = faker.random.locale();
+    const currency = faker.finance.currencyCode();
+    const country = faker.address.country();
+    const timezone = faker.lorem.word();
+
+    await extract(
+      <I18nUniversalProvider
+        locale={locale}
+        currency={currency}
+        timezone={timezone}
+        country={country}
+      />,
+      {
+        decorate: (element: React.ReactNode) => (
+          <HtmlContext.Provider value={htmlManager}>
+            {element}
+          </HtmlContext.Provider>
+        ),
+      },
+    );
+
+    const overrideDetails = {
+      locale: faker.random.locale(),
+      currency: undefined,
+      country: faker.address.country(),
+      timezone: undefined,
+    };
+    const i18n = mount(<I18nUniversalProvider {...overrideDetails} />, {
+      htmlManager,
+    });
+
+    expect(i18n).toProvideReactContext(
+      I18nContext,
+      expect.objectContaining({
+        details: expect.objectContaining({
+          locale: overrideDetails.locale,
+          timezone,
+          currency,
+          country: overrideDetails.country,
+        }),
+      }),
+    );
+  });
 });

packages/react-i18n-universal-provider/src/test/utilities.test.ts

@@ -0,0 +1,91 @@
+import faker from 'faker';
+
+import {combinedI18nDetails} from '../utilities';
+
+describe('combinedI18nDetails()', () => {
+  it('merges two details objects together', () => {
+    const details = {
+      locale: faker.random.locale(),
+      country: faker.address.country(),
+    };
+    const overrides = {
+      currency: faker.finance.currencyCode(),
+    };
+
+    const combinedDetails = combinedI18nDetails(details, overrides);
+
+    expect(combinedDetails).toHaveProperty('locale');
+    expect(combinedDetails).toHaveProperty('country');
+    expect(combinedDetails).toHaveProperty('currency');
+  });
+
+  it('overrides fields with defined overrides', () => {
+    const locale = faker.random.locale();
+    const country = faker.address.country();
+    const currency = faker.finance.currencyCode();
+    const details = {
+      locale,
+      country,
+      currency,
+    };
+    const overrideCurrency = faker.finance.currencyCode();
+    const overrides = {
+      locale: undefined,
+      country: undefined,
+      currency: overrideCurrency,
+    };
+
+    const combinedDetails = combinedI18nDetails(details, overrides);
+
+    expect(combinedDetails).toHaveProperty('locale', locale);
+    expect(combinedDetails).toHaveProperty('country', country);
+    expect(combinedDetails).toHaveProperty('currency', overrideCurrency);
+  });
+
+  describe('locale', () => {
+    it('returns a details object with a locale field', () => {
+      const locale = faker.random.locale();
+      const fallbackLocale = faker.random.locale();
+      const details = {locale};
+      const overrides = {locale: undefined, fallbackLocale};
+
+      expect(combinedI18nDetails(details, overrides)).toHaveProperty(
+        'locale',
+        locale,
+      );
+    });
+
+    it('favours an override locale if one is specified', () => {
+      const locale = faker.random.locale();
+      const overrideLocale = faker.random.locale();
+      const details = {locale};
+      const overrides = {locale: overrideLocale};
+
+      expect(combinedI18nDetails(details, overrides)).toHaveProperty(
+        'locale',
+        overrideLocale,
+      );
+    });
+
+    it('returns a details object with a fallback locale field if one is specified', () => {
+      const fallbackLocale = faker.random.locale();
+      const details = {};
+      const overrides = {fallbackLocale};
+
+      expect(combinedI18nDetails(details, overrides)).toHaveProperty(
+        'locale',
+        fallbackLocale,
+      );
+    });
+
+    it('returns a details object with a default locale field if none is specified', () => {
+      const details = {};
+      const overrides = {};
+
+      expect(combinedI18nDetails(details, overrides)).toHaveProperty(
+        'locale',
+        'en',
+      );
+    });
+  });
+});

packages/react-i18n-universal-provider/src/utilities.ts

@@ -0,0 +1,17 @@
+function pruneUndefinedFields(obj = {}) {
+  const objCopy = {...obj};
+
+  Object.keys(objCopy).forEach(
+    key => objCopy[key] === undefined && delete objCopy[key],
+  );
+
+  return objCopy;
+}
+
+export function combinedI18nDetails(details, overrides) {
+  return {
+    locale: overrides.fallbackLocale || 'en',
+    ...pruneUndefinedFields(details),
+    ...pruneUndefinedFields(overrides),
+  };
+}