Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix number replacement value returning an array instead of a string #2292

Merged
merged 2 commits into from
May 30, 2022

Conversation

emileber
Copy link
Contributor

@emileber emileber commented May 30, 2022

Description

A regression was introduced following:

It was identified in another project where a test case expecting a string was failing since it was getting an array with the new @shopify/react-i18n version 7.1.0:

Expected: "Cannot be greater than 200 characters, current is 205 characters"
Received: ["Cannot be greater than ", 200, " characters, current is ", 205, " characters"]

Since this function is a little complex with a different return value type based on values inside the replacement dictionary, I've added 2 tests that validates the return value of translate.

That way, I caught another issue where an empty string was returned in the array when passing a React element as the last replacement value.

Type of change

  • @shopify/react-i18n Patch: Bug (non-breaking change which fixes an issue)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above (Documentation fix and Test update does not need a changelog as we do not publish new version)

@emileber emileber marked this pull request as ready for review May 30, 2022 05:31
@emileber emileber requested a review from a team as a code owner May 30, 2022 05:31
@emileber emileber merged commit 7e52f83 into main May 30, 2022
@emileber emileber deleted the emileber/fix-react-i18n-regression branch May 30, 2022 14:44
@shopify-shipit shopify-shipit bot temporarily deployed to production-js May 30, 2022 20:34 Inactive
@BPScott
Copy link
Member

BPScott commented May 30, 2022

It seems that this was not a complete fix: https://buildkite.com/shopify/web-ci-builder/builds/482401

@shopify-shipit shopify-shipit bot temporarily deployed to beta-js June 1, 2022 16:44 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to beta-js-apollo June 8, 2022 13:04 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production-gem June 21, 2022 14:45 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants