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

Select Macro Documentation Example is wrong #1324

Closed
thekip opened this issue Jan 6, 2023 · 3 comments · Fixed by #1421
Closed

Select Macro Documentation Example is wrong #1324

thekip opened this issue Jan 6, 2023 · 3 comments · Fixed by #1421

Comments

@thekip
Copy link
Collaborator

thekip commented Jan 6, 2023

Example from documentation:

import { Select } from "@lingui/macro"

// gender == "female"      -> Her book
// gender == "male"        -> His book
// gender == "unspecified" -> Their book
<Select
    value={gender}
    male="His book"
    female="Her book"
    other="Their book"
/>

Actually, it should be:

import { Select } from "@lingui/macro"

<Select
    value={gender}
    _male="His book"
    _female="Her book"
    other="Their book"
/>

Proof: https://github.com/lingui/js-lingui/blob/main/packages/macro/test/jsx-select.ts#L3-L19

If you write code as in example, macro will produce:

import { Trans } from "@lingui/react";
<Trans
+  male="He"
+  female={`She`}
  id={"{count, select, male {He} female {She} other {<0>Other</0>}}"}
  values={{
    count: count,
  }}
  components={{
    0: <strong />,
  }}
/>;

Which by the way will work, but unnecessary code would be produced.

So here we should either fix the macro to not produce this fields and add this to the test case.
Or update docs and fix usage with _{option} which would be a breaking change.

Anyway, action point needed here.

@thekip thekip mentioned this issue Jan 6, 2023
@thekip
Copy link
Collaborator Author

thekip commented Jan 6, 2023

Looked at the code one more time, and looks like this work by mistake. The original author's intention was to have exact choices with underscore only. Here is the regex:

/(_[\d\w]+|zero|one|two|few|many|other)/

@thekip
Copy link
Collaborator Author

thekip commented Jan 14, 2023

Related #1288

@tricoder42
Copy link
Contributor

Looked at the code one more time, and looks like this work by mistake. The original author's intention was to have exact choices with underscore only. Here is the regex:

/(_[\d\w]+|zero|one|two|few|many|other)/

You're right. ICU MessageFormat uses = for exact numbers in plural-like formats (e.g. =0). I replaced = with _ since you can't use = character in prop names. I believe I kept the same naming convention in select format a) to keep it consistent b) to avoid name clashes with macro props.

thekip added a commit to thekip/js-lingui that referenced this issue Feb 9, 2023
@andrii-bodnar andrii-bodnar linked a pull request Feb 9, 2023 that will close this issue
7 tasks
@andrii-bodnar andrii-bodnar modified the milestone: v4 Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants