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

feat: add setMessagesCompiler method #2035

Merged
merged 5 commits into from
Oct 14, 2024
Merged

Conversation

timofei-iatsenko
Copy link
Collaborator

@timofei-iatsenko timofei-iatsenko commented Sep 21, 2024

Description

  1. Allow users to use uncompiled catalogs on production (json)
  2. have a proper working fallback to default message defined in the code (with the proper macro config)

Related user request: #2012

To achieve these goals, i18n.setMessagesCompiler() method was provided, which allow to manually register message compiler on production.

Also, the format of compiled message was slightly changed.

Previously, if the message doesn't have any ICU features inside it was printed as a string, array used for the rest of cases. That make distinguishing between compiled and uncompiled message particularly hard and sometimes lead to double compilation which was tried to be fixed here #1913

Now uncompiled message is always a string, and compiled is always an array.

Some examples:

"Hey '{name}'!" -> ["Hey {name}!"] // escaped placeholder
"Hey {name}!" -> ["Hey", "name", "!"] // icu placeholder
"Hey!" -> ["Hey!"] 

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Examples update

Fixes # (issue)

Checklist

  • I have read the CONTRIBUTING and CODE_OF_CONDUCT docs
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation (if appropriate)

Copy link

vercel bot commented Sep 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
js-lingui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 4, 2024 10:02am

Copy link

github-actions bot commented Sep 21, 2024

size-limit report 📦

Path Size
./packages/core/dist/index.mjs 2.88 KB (0%)
./packages/detect-locale/dist/index.mjs 723 B (0%)
./packages/react/dist/index.mjs 1.65 KB (0%)
./packages/remote-loader/dist/index.mjs 7.26 KB (0%)

@timofei-iatsenko
Copy link
Collaborator Author

Had to exclude test from @lingui/remote-loader, because it's not published since version 3 and has outdated dependencies. That causes integration tests to fail, because snapshots are created using source code and new compiler format, but integration tests use packages from node_modules and for @lingui/remote-loader it takes stale versions.

@timofei-iatsenko
Copy link
Collaborator Author

Something happened to Verdaccio. The WF uses:

npm i -g {verdaccio,verdaccio-auth-memory,verdaccio-memory}

Which is installing latest possible version, they published version 6 couple days ago and that might be the reason. I tried to fix v5 version, but it didn't help. The error:

info attempt registry request try #1 at 10:04:12 AM
http request PUT http://0.0.0.0:4873/-/user/org.couchdb.user:test
info retry will retry, error on last attempt: Error: connect ECONNREFUSED 0.0.0.0:4873

However, the verdaccio itself is up and running:

image

@timofei-iatsenko timofei-iatsenko marked this pull request as ready for review September 22, 2024 10:27
.github/workflows/release-test.yml Outdated Show resolved Hide resolved
packages/core/src/i18n.ts Show resolved Hide resolved
vonovak

This comment was marked as outdated.

@vonovak vonovak self-requested a review September 29, 2024 18:33
Copy link
Collaborator

@vonovak vonovak left a comment

Choose a reason for hiding this comment

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

Hi, apologies for late reply. I have the same feedback as Andrii - docs :)

@timofei-iatsenko
Copy link
Collaborator Author

@andrii-bodnar @vonovak i added docs for that method. The better overall tutorial will be in next iterations after this epic finished

website/docs/ref/core.md Show resolved Hide resolved
Example usage:

```ts
import { compileMessage } from "@lingui/message-utils/compileMessage";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
import { compileMessage } from "@lingui/message-utils/compileMessage";
// `compileMessage` is Lingui's default implementation
import { compileMessage } from "@lingui/message-utils/compileMessage";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's coming from @lingui/ scope, isn't it obvious enough? More over, this method wasn't built to make it possible to override a default implementation, so it's intended to use always with a default implementation.

Copy link
Collaborator

@vonovak vonovak Oct 9, 2024

Choose a reason for hiding this comment

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

this method wasn't built to make it possible to override a default implementation, so it's intended to use always with a default implementation.

That's a bit confusing to me because then I don't understand the use of this so much. But it's probably because other changes need to be done before this can be properly documented, as you said before.

isn't it obvious enough?

Well, just a few moments ago it felt obvious to me that I can use also something else other than the default implementation, so the word "obvious" can be a bit tricky.

If no other implementation should be used, the docs should say that imho.

Anyways, I don't want to block this PR from merging - I've raised the points I wanted to. 🙂👍
I'll defere to @andrii-bodnar

website/docs/ref/core.md Show resolved Hide resolved
@andrii-bodnar andrii-bodnar merged commit baec0ab into next Oct 14, 2024
12 checks passed
@andrii-bodnar andrii-bodnar deleted the feat/set-message-compiler branch October 14, 2024 13:26
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.

3 participants