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

fix: expression only choices / select options #1523

Merged
merged 2 commits into from
Mar 16, 2023
Merged

fix: expression only choices / select options #1523

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d517e14
fix: support pure expression choices
taozhou-glean Mar 16, 2023
d39ca42
update tests and add a few for jsx
taozhou-glean Mar 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions packages/macro/src/macroJs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,47 @@ describe("js macro", () => {
})
})

it("plural with variable only cases", () => {
const macro = createMacro()
const exp = parseExpression(
"plural(count, { one: `# glass of ${drink}`, other: drink})"
)
const tokens = macro.tokenizeChoiceComponent(exp as CallExpression)
expect(tokens).toEqual({
type: "arg",
name: "count",
value: expect.objectContaining({
name: "count",
type: "Identifier",
}),
format: "plural",
options: {
one: [
{
type: "text",
value: "# glass of ",
},
{
type: "arg",
name: "drink",
value: expect.objectContaining({
name: "drink",
type: "Identifier",
}),
},
],
other: {
type: "arg",
name: "drink",
value: expect.objectContaining({
name: "drink",
type: "Identifier",
}),
},
},
})
})

taozhou-glean marked this conversation as resolved.
Show resolved Hide resolved
it("plural with select", () => {
const macro = createMacro()
const exp = parseExpression(
Expand Down Expand Up @@ -343,4 +384,47 @@ describe("js macro", () => {
})
})
})

it("select with expression", () => {
const macro = createMacro()
const exp = parseExpression(
"select(gender, {male: hePronoun, female: 'she', other: 'they'})"
)
const tokens = macro.tokenizeChoiceComponent(exp as CallExpression)
expect(tokens).toEqual({
format: "select",
name: "gender",
options: expect.objectContaining({
female: "she",
male: expect.objectContaining({
type: "arg",
name: "hePronoun",
value: expect.objectContaining({
name: "hePronoun",
type: "Identifier",
}),
}),
offset: undefined,
other: "they",
}),
type: "arg",
value: {
end: 13,
loc: {
end: expect.objectContaining({
column: 13,
line: 1,
}),
identifierName: "gender",
start: expect.objectContaining({
column: 7,
line: 1,
}),
},
name: "gender",
start: 7,
type: "Identifier",
},
})
})
})
6 changes: 5 additions & 1 deletion packages/macro/src/macroJs.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,12 @@ export default class MacroJs {
value = this.tokenizeTemplateLiteral(attrValue)
} else if (this.types.isCallExpression(attrValue)) {
value = this.tokenizeNode(attrValue)
} else if (this.types.isStringLiteral(attrValue)) {
value = attrValue.value
} else if (this.types.isExpression(attrValue)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

StringLiteral is also considered as Expression, so I moved StringLiteral check above. From ts, after all the checks, the types fall through are limited to babelTypes.RestElement | babelTypes.AssignmentPattern | babelTypes.ArrayPattern | babelTypes.ObjectPattern, not sure exactly how we want to handle them, so I kept the previous fallback case untouched.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the unknown conversion is just to make ts happy, please let me know if you have other suggestions to handle the remaining cases.

value = this.tokenizeExpression(attrValue)
} else {
value = (attrValue as StringLiteral).value
value = (attrValue as unknown as StringLiteral).value
}
token.options[name] = value
}
Expand Down
26 changes: 26 additions & 0 deletions packages/macro/test/js-plural.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,31 @@ const cases: TestCase[] = [
);
`,
},
{
name: "Macro with expression only choice",
input: `
import { plural } from '@lingui/macro'
plural(users.length, {
offset: 1,
0: "No books",
1: "1 book",
other: someOtherExp
});
`,
expected: `
import { i18n } from "@lingui/core";
i18n._(
/*i18n*/
{
id: "0mcXIe",
message: "{0, plural, offset:1 =0 {No books} =1 {1 book} other {{someOtherExp}}}",
values: {
0: users.length,
someOtherExp: someOtherExp,
},
}
);
`,
},
]
export default cases
29 changes: 29 additions & 0 deletions packages/macro/test/js-select.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,34 @@ const cases: TestCase[] = [
);
`,
},
{
name: "Nested macros with pure expressions option",
input: `
import { select, plural } from '@lingui/macro'
select(gender, {
"male": plural(numOfGuests, {
one: "He invites one guest",
other: "He invites # guests"
}),
female: \`She is \${gender}\`,
other: someOtherExp
});
`,
expected: `
import { i18n } from "@lingui/core";
i18n._(
/*i18n*/
{
id: "j9PNNm",
message: "{gender, select, male {{numOfGuests, plural, one {He invites one guest} other {He invites # guests}}} female {She is {gender}} other {{someOtherExp}}}",
values: {
gender: gender,
numOfGuests: numOfGuests,
someOtherExp: someOtherExp,
},
}
);
`,
},
]
export default cases