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: Make pseudolocal use the message AST instead of the key #1293

Merged
merged 2 commits into from
Nov 7, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 3 additions & 1 deletion packages/cli/src/api/__snapshots__/compile.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ Can't parse message. Please check correct syntax: "{value, plural, one {Book} ot
Messageformat-parser trace: Expected "#", "{", "}", doubled apostrophe, escaped string, or plain char but end of input found.
`;

exports[`createCompiledCatalog nested message 1`] = `/*eslint-disable*/module.exports={messages:{"nested":{"one":"Uno","two":"Dos","three":"Tres","hello":["Hola ",["name"]]}}};`;

exports[`createCompiledCatalog options.compilerBabelOptions by default should return catalog without ASCII chars 1`] = `/*eslint-disable*/module.exports={messages:{"Hello":"Alohà"}};`;

exports[`createCompiledCatalog options.compilerBabelOptions should return catalog without ASCII chars 1`] = `/*eslint-disable*/module.exports={messages:{"Hello":"Aloh\\xE0"}};`;
Expand All @@ -20,7 +22,7 @@ exports[`createCompiledCatalog options.namespace should compile with window 1`]

exports[`createCompiledCatalog options.namespace should error with invalid value 1`] = `Invalid namespace param: "global"`;

exports[`createCompiledCatalog options.pseudoLocale should return catalog with pseudolocalized messages 1`] = `/*eslint-disable*/module.exports={messages:{"Hello":"Ĥēĺĺō"}};`;
exports[`createCompiledCatalog options.pseudoLocale should return catalog with pseudolocalized messages 1`] = `/*eslint-disable*/module.exports={messages:{"Hello":"ÀĥōĴ"}};`;

exports[`createCompiledCatalog options.pseudoLocale should return compiled catalog when pseudoLocale doesn't match current locale 1`] = `/*eslint-disable*/module.exports={messages:{"Hello":"Ahoj"}};`;

Expand Down
174 changes: 164 additions & 10 deletions packages/cli/src/api/compile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ import generate from "@babel/generator"
import { compile, createCompiledCatalog } from "./compile"

describe("compile", () => {
const getSource = (message) =>
generate(compile(message) as any, {
const getSource = (message: string, shouldPseudolocalize: boolean = false) =>
generate(compile(message, shouldPseudolocalize) as any, {
compact: true,
minified: true,
jsescOption: { minimal: true },
}).code

it("should optimize string only messages", () => {
Expand Down Expand Up @@ -67,14 +68,165 @@ describe("compile", () => {
)
})

it("should compile multiple plurals", () => {
expect(
getSource(
"{bcount, plural, one {boy} other {# boys}} {gcount, plural, one {girl} other {# girls}}"
)
).toEqual(
'[["bcount","plural",{one:"boy",other:["#"," boys"]}]," ",["gcount","plural",{one:"girl",other:["#"," girls"]}]]'
)
})

it("should report failed message on error", () => {
expect(() =>
getSource("{value, plural, one {Book} other {Books")
).toThrowErrorMatchingSnapshot()
})

describe("with pseudo-localization", () => {
const getPSource = (message: string) => getSource(message, true)

it("should pseudolocalize strings", () => {
expect(getPSource("Martin Černý")).toEqual('"Màŕţĩń Čēŕńý"')
})

it("should pseudolocalize escaping syntax characters", () => {
// TODO: should this turn into pseudoLocale string?
expect(getSource("'{name}'", true)).toEqual('"{name}"')
// expect(getSource("'{name}'", true)).toEqual('"{ńàmē}"')
})

it("should not pseudolocalize arguments", () => {
expect(getPSource("{name}")).toEqual('[["name"]]')
expect(getPSource("B4 {name} A4")).toEqual('["ß4 ",["name"]," À4"]')
})

it("should not pseudolocalize arguments nor formats", () => {
expect(getPSource("{name, number}")).toEqual('[["name","number"]]')
expect(getPSource("{name, number, percent}")).toEqual(
'[["name","number","percent"]]'
)
})

it("should not pseudolocalize HTML tags", () => {
expect(getPSource('Martin <span id="spanId">Černý</span>')).toEqual(
'"Màŕţĩń <span id=\\"spanId\\">Čēŕńý</span>"'
)
sharonyogev marked this conversation as resolved.
Show resolved Hide resolved
expect(
getPSource("Martin Cerny 123a<span id='id'>Černý</span>")
).toEqual('"Màŕţĩń Ćēŕńŷ 123à<span id=\'id\'>Čēŕńý</span>"')
expect(getPSource("Martin <a title='>>a'>a</a>")).toEqual(
sharonyogev marked this conversation as resolved.
Show resolved Hide resolved
'"Màŕţĩń <a title=\'>>a\'>à</a>"'
)
expect(getPSource("<a title=TITLE>text</a>")).toEqual(
'"<a title=TITLE>ţēxţ</a>"'
)
})

describe("Plurals", () => {
it("with value", () => {
expect(
getPSource("{value, plural, one {# book} other {# books}}")
).toEqual('[["value","plural",{one:["#"," ƀōōķ"],other:["#"," ƀōōķś"]}]]')
})

it("with variable placeholder", () => {
expect(
getPSource(
"{count, plural, one {{countString} book} other {{countString} books}}"
)
).toEqual(
'[["count","plural",{one:[["countString"]," ƀōōķ"],other:[["countString"]," ƀōōķś"]}]]'
)
})

it("with offset", () => {
expect(
getPSource(
"{count, plural, offset:1 zero {There are no messages} other {There are # messages in your inbox}}"
)
).toEqual(
'[["count","plural",{offset:1,zero:"Ţĥēŕē àŕē ńō mēśśàĝēś",other:["Ţĥēŕē àŕē ","#"," mēśśàĝēś ĩń ŷōũŕ ĩńƀōx"]}]]'
)
})

it("with HTML tags", () => {
expect(
getPSource(
"{count, plural, zero {There's # <span>message</span>} other {There are # messages}}"
)
).toEqual(
'[["count","plural",{zero:["Ţĥēŕē\'ś ","#"," <span>mēśśàĝē</span>"],other:["Ţĥēŕē àŕē ","#"," mēśśàĝēś"]}]]'
)
})

it("with exact number", () => {
expect(
getPSource(
"{count, plural, =0 {There's # <span>message</span>} other {There are # messages}}"
)
).toEqual(
'[["count","plural",{0:["Ţĥēŕē\'ś ","#"," <span>mēśśàĝē</span>"],other:["Ţĥēŕē àŕē ","#"," mēśśàĝēś"]}]]'
)
})
})

it("SelectOrdinal", () => {
expect(
getPSource(
"{count, selectordinal, offset:1 one {#st} two {#nd} few {#rd} =4 {4th} many {testMany} other {#th}}"
)
).toEqual(
'[["count","selectordinal",{offset:1,one:["#","śţ"],two:["#","ńď"],few:["#","ŕď"],4:"4ţĥ",many:"ţēśţMàńŷ",other:["#","ţĥ"]}]]'
)
})

it("Select", () => {
expect(
getPSource(
"{gender, select, male {He} female {She} other {<span>Other</span>}}"
)
).toEqual(
'[["gender","select",{male:"Ĥē",female:"Śĥē",other:"<span>Ōţĥēŕ</span>"}]]'
)
})

it("should not pseudolocalize variables", () => {
expect(getPSource("replace {count}")).toEqual('["ŕēƥĺàćē ",["count"]]')
expect(getPSource("replace { count }")).toEqual('["ŕēƥĺàćē ",["count"]]')
})

it("Multiple Plurals", () => {
expect(
getPSource(
"{bcount, plural, one {boy} other {# boys}} {gcount, plural, one {girl} other {# girls}}"
)
).toEqual(
'[["bcount","plural",{one:"ƀōŷ",other:["#"," ƀōŷś"]}]," ",["gcount","plural",{one:"ĝĩŕĺ",other:["#"," ĝĩŕĺś"]}]]'
)
})
})
})

describe("createCompiledCatalog", () => {
it("nested message", () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just added this test for more coverage on this file

expect(
createCompiledCatalog(
"cs",
{
nested: {
one: "Uno",
two: "Dos",
three: "Tres",
hello: "Hola {name}",
},
},
{}
)
).toMatchSnapshot()
})

describe("options.namespace", () => {
const getCompiledCatalog = (namespace) =>
createCompiledCatalog(
Expand Down Expand Up @@ -113,7 +265,7 @@ describe("createCompiledCatalog", () => {
{
Hello: "Ahoj",
Missing: "",
Select: "{id, select, Gen {Genesis} 1John {1 John} other {____}}"
Select: "{id, select, Gen {Genesis} 1John {1 John} other {____}}",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prettier changes (this and the following)

},
{
strict,
Expand Down Expand Up @@ -165,13 +317,15 @@ describe("createCompiledCatalog", () => {
})

it("should return catalog without ASCII chars", () => {
expect(getCompiledCatalog({
compilerBabelOptions: { 
jsescOption: {
minimal: false,
}
}
})).toMatchSnapshot()
expect(
getCompiledCatalog({
compilerBabelOptions: {
jsescOption: {
minimal: false,
},
},
})
).toMatchSnapshot()
})
})
})
27 changes: 14 additions & 13 deletions packages/cli/src/api/compile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import * as R from "ramda"

import pseudoLocalize from "./pseudoLocalize"


const INVALID_OBJECT_KEY_REGEX = /^(\d+[a-zA-Z]|[a-zA-Z]+\d)(\d|[a-zA-Z])*/
export type CompiledCatalogNamespace = "cjs" | "es" | "ts" | string

Expand All @@ -26,11 +25,7 @@ export type CreateCompileCatalogOptions = {
* applying pseudolocalization where necessary.
*/
function compileSingleKey(key: string, translation: string, shouldPseudolocalize: boolean): t.ObjectProperty {
if (shouldPseudolocalize) {
translation = pseudoLocalize(key)
}

return t.objectProperty(t.stringLiteral(key), compile(translation))
return t.objectProperty(t.stringLiteral(key), compile(translation, shouldPseudolocalize))
}

export function createCompiledCatalog(
Expand Down Expand Up @@ -118,34 +113,40 @@ function buildExportStatement(expression, namespace: CompiledCatalogNamespace) {
* Compile string message into AST tree. Message format is parsed/compiled into
* JS arrays, which are handled in client.
*/
export function compile(message: string) {
export function compile(message: string, shouldPseudolocalize: boolean = false) {
let tokens

try {
tokens = parse(message)
} catch (e) {
throw new Error(
`Can't parse message. Please check correct syntax: "${message}" \n \n Messageformat-parser trace: ${e.message}`,
`Can't parse message. Please check correct syntax: "${message}" \n \n Messageformat-parser trace: ${e.message}`
)
}
const ast = processTokens(tokens)
const ast = processTokens(tokens, shouldPseudolocalize)

if (isString(ast)) return t.stringLiteral(ast)

return ast
}

function processTokens(tokens) {
function processTokens(tokens, shouldPseudolocalize: boolean) {
// Shortcut - if the message doesn't include any formatting,
// simply join all string chunks into one message
if (!tokens.filter((token) => !isString(token)).length) {
sharonyogev marked this conversation as resolved.
Show resolved Hide resolved
return tokens.join("")
if (shouldPseudolocalize) {
return tokens.map((token) => pseudoLocalize(token)).join("")
} else {
return tokens.join("")
}
}

return t.arrayExpression(
tokens.map((token) => {
if (isString(token)) {
return t.stringLiteral(token)
return t.stringLiteral(
shouldPseudolocalize ? pseudoLocalize(token) : token
)

// # in plural case
} else if (token.type === "octothorpe") {
Expand Down Expand Up @@ -179,7 +180,7 @@ function processTokens(tokens) {
}

token.cases.forEach((item) => {
const inlineTokens = processTokens(item.tokens)
const inlineTokens = processTokens(item.tokens, shouldPseudolocalize)
formatProps.push(
t.objectProperty(
// if starts with number must be wrapped with quotes
Expand Down
18 changes: 14 additions & 4 deletions packages/cli/src/api/pseudoLocalize.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,20 @@ describe("PseudoLocalization", () => {
it("with HTML tags", () => {
expect(
pseudoLocalize(
"{count, plural, zero {There's # <span>message</span>} other {There are # messages}"
"{count, plural, zero {There's # <span>message</span>} other {There are # messages}}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these weren't valid ICU messages (it does not make the tests failed, since these tests does not validate it's a valid ICU message), but still need to be correct (unless we'll remove the ICU handling from this file)

)
).toEqual(
"{count, plural, zero {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}"
"{count, plural, zero {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}}"
)
})

it("with exact number", () => {
expect(
pseudoLocalize(
"{count, plural, =0 {There's # <span>message</span>} other {There are # messages}"
"{count, plural, =0 {There's # <span>message</span>} other {There are # messages}}"
)
).toEqual(
"{count, plural, =0 {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}"
"{count, plural, =0 {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}}"
)
})
})
Expand Down Expand Up @@ -92,4 +92,14 @@ describe("PseudoLocalization", () => {
expect(pseudoLocalize("replace {count}")).toEqual("ŕēƥĺàćē {count}")
expect(pseudoLocalize("replace { count }")).toEqual("ŕēƥĺàćē { count }")
})

it("multiple plurals is wrong45", () => {
expect(
pseudoLocalize(
"{bcount, plural, one {boy} other {# boys}} {gcount, plural, one {girl} other {# girls}}"
)
).not.toEqual(
"{bcount, plural, one {ƀōŷ} other {# ƀōŷś}} {gcount, plural, one {ĝĩŕĺ} other {# ĝĩŕĺś}}"
)
})
})
sharonyogev marked this conversation as resolved.
Show resolved Hide resolved