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.

Sign up for GitHub

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

Jump to bottom

feat(i18n): allow custom interpolation options for i18n #4727

Merged
merged 4 commits into from
May 29, 2024
Merged

feat(i18n): allow custom interpolation options for i18n #4727

merged 4 commits into from
May 29, 2024
+328 −14

Conversation

zleight1
Copy link
Contributor

Allow for specifying custom interpolation options for i18n. This is useful for custom interpolation patterns, such as {{ and }}. Tests added, and scoped to the i18n module.

Closes #4726

🔎 Overview

Adds the ability to use custom interpolation options, such as prefix and suffix.

This allows the library to comply with the default i18next standard.

🤓 Code snippets/examples (if applicable)

configure({
  generateMessage: localize(
    'en',
    {
      messages: {
        between: 'The {{field}} field must be between 0:{{min}} and 1:{{max}}',
      },
    },
    {
      prefix: '{{',
      suffix: '}}',
    },
  ),
});

✔ Issues affected

closes #4726

@zleight1
feat(i18n): allow custom interpolation options for i18n
75e229f 
Allow for specifying custom interpolation options for i18n.
This is useful for custom interpolation patterns, such as `{{` and `}}`.
Tests added, and scoped to the i18n module.

Closes logaretm#4726
@codecov Codecov
Copy link

codecov bot commented Apr 15, 2024 •
edited

Codecov Report

Attention: Patch coverage is 91.48936% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 89.89%. Comparing base (2a0881f) to head (ebece65).
Report is 13 commits behind head on main.

Files Patch % Lines
packages/i18n/src/index.ts 90.32% 3 Missing ⚠️
packages/i18n/src/utils.ts 90.90% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #4727      +/-   ##
==========================================
+ Coverage   89.87%   89.89%   +0.01%     
==========================================
  Files          93       93              
  Lines        7676     7709      +33     
  Branches     1360     1362       +2     
==========================================
+ Hits         6899     6930      +31     
- Misses        770      772       +2     
  Partials        7        7

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zleight1
Copy link
Contributor Author

If accepted I'll update the docs as well

@zleight1
Copy link
Contributor Author

@logaretm - is there something I can do to help with making this a reality?

If there's some reason you would not want this, please let me know why so I can understand more.

Thanks!

logaretm
logaretm requested changes May 28, 2024
View reviewed changes
Copy link
Owner

@logaretm logaretm left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, Thank you @zleight1, This looks alright to me. I have no concerns here other than the test file seems to have changed significantly. I would have assumed this would need to add a test case or two but the changes here are too significant with some tests altered, if possible lets keep everything as-is and only add the needed test cases for this interpolation feature.

Once that is fixed we can merge this right away.

packages/i18n/tests/index.spec.ts Outdated Show resolved Hide resolved
@zleight1
test(i18n): undo describe scoping
ee97884 
Unwrapped the tests from the describe blocks, so the diff is cleaner.
Prefixed new tests names so they don't collide, also in commented section with begin/end.

logaretm#4726
@zleight1
docs(i18n): add docs for interpolation settings
7ff832c 
logaretm#4726
@zleight1
Copy link
Contributor Author

If accepted I'll update the docs as well

Updated this as well, let me know if it needs anything else.

@zleight1 zleight1 requested a review from logaretm May 29, 2024 14:28
logaretm
logaretm approved these changes May 29, 2024
View reviewed changes
Copy link
Owner

@logaretm logaretm left a comment

Choose a reason for hiding this comment

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

Amazing, thank you for addressing my nitpicks 😛

Thanks a lot for this, merging right away and will tag a release once another PR is merged.

@logaretm logaretm merged commit dadc58a into logaretm:main May 29, 2024
7 checks passed
@zleight1
Copy link
Contributor Author

Amazing, thank you for addressing my nitpicks 😛

Thanks a lot for this, merging right away and will tag a release once another PR is merged.

I'm all for the nitpicks, thanks for the guidance!

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

@logaretm logaretm logaretm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ability to customize interpolation prefix/suffix
2 participants
@zleight1 @logaretm