Skip to content

Commit

Permalink
Only insert key on react elements (#188)
Browse files Browse the repository at this point in the history
* Only format count/ordinal as number if they are one

* changeset

* Declare explicit peer dep on React

We need to use utilities such as cloneElement and isValidElement

* Clone element when necessary to inject key

We're re-creating any object that doesn't have a key and injecting a key
into it, which is causing non-react elements such as arrays and
`expect.any` style matchers to be mutated into a shape that can't then
be used.

Arrays are valid as children and should be preserved, and expect.any is
useful for matching translations.

We should be using React's cloneElement when we need to mutate an
element in this way so that it can still be tested against when
injecting a key prop into it.

* changeset
  • Loading branch information
ryanwilsonperkin authored Sep 14, 2023
1 parent 6fd7800 commit cf07b33
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/breezy-poets-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/i18next-shopify': patch
---

Don't break objects when inserting key prop
5 changes: 5 additions & 0 deletions .changeset/breezy-pugs-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/i18next-shopify': patch
---

Only format count/ordinal as number if they are one
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
"type": "git",
"url": "https://github.com/Shopify/i18next-shopify"
},
"peerDependencies": {
"react": "*"
},
"devDependencies": {
"@babel/cli": "^7.15.0",
"@babel/core": "^7.15.0",
Expand Down
7 changes: 4 additions & 3 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const {isValidElement, cloneElement} = require('react');

const arr = [];
const each = arr.forEach;

Expand Down Expand Up @@ -34,10 +36,9 @@ export function replaceValue(interpolated, pattern, replacement) {
if (split.length !== 1 && typeof replacement === 'object') {
// Return array w/ the replacement

// React elements within arrays need a key prop
if (!replacement.key) {
if (!replacement.key && isValidElement(replacement)) {
// eslint-disable-next-line no-param-reassign
replacement = {...replacement, key: pattern.toString()};
replacement = cloneElement(replacement, {key: pattern.toString()});
}

return [split[0], replacement, split[1]].flat();
Expand Down
23 changes: 23 additions & 0 deletions test/shopify_format_with_react_i18next.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,29 @@ describe('shopify format with react-i18next (t)', () => {
informal_greeting: 'Sup Joe',
});
});

it('matches expect.anything() in interpolated variables', () => {
const {result} = renderHook(() => useTranslation('translation'));
const {t} = result.current;

expect(
t('string_with_single_mustache', {
name: <strong>Joe</strong>,
}),
).toStrictEqual(
t('string_with_single_mustache', {name: expect.anything()}),
);
});

it('accepts arrays as interpolated variables', () => {
const {result} = renderHook(() => useTranslation('translation'));
const {t} = result.current;
expect(
t('string_with_single_mustache', {
name: ['Joe', 'Jane'],
}),
).toStrictEqual(['Hello ', 'Joe', 'Jane', '!']);
});
});

describe('with react-i18next (Trans)', () => {
Expand Down

0 comments on commit cf07b33

Please sign in to comment.