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(cli): {locale} and {name} replace only once in catalog path #1342

Merged
merged 1 commit into from
Jan 18, 2023
Merged

fix(cli): {locale} and {name} replace only once in catalog path #1342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
117 changes: 84 additions & 33 deletions packages/cli/src/api/catalog.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe("Catalog", function () {
})

describe("POT Flow", function () {
it('Should merge source messages from template if provided', () => {
it("Should merge source messages from template if provided", () => {
const catalog = new Catalog(
{
name: "messages",
Expand All @@ -144,21 +144,21 @@ describe("Catalog", function () {
exclude: [],
},
mockConfig({
locales: ['en', 'pl'],
locales: ["en", "pl"],
})
)

const translations = catalog.getTranslations('pl', {
sourceLocale: 'en',
const translations = catalog.getTranslations("pl", {
sourceLocale: "en",
fallbackLocales: {
default: 'en'
}
});
default: "en",
},
})

expect(translations).toMatchSnapshot()
})

it('Should get translations from template if locale file not presented', () => {
it("Should get translations from template if locale file not presented", () => {
const catalog = new Catalog(
{
name: "messages",
Expand All @@ -170,18 +170,18 @@ describe("Catalog", function () {
exclude: [],
},
mockConfig({
locales: ['en', 'pl'],
locales: ["en", "pl"],
})
)

const translations = catalog.getTranslations('en', {
sourceLocale: 'en',
const translations = catalog.getTranslations("en", {
sourceLocale: "en",
fallbackLocales: {
default: 'en'
}
});
default: "en",
},
})

console.log(translations);
console.log(translations)
expect(translations).toMatchSnapshot()
})
})
Expand All @@ -207,15 +207,18 @@ describe("Catalog", function () {
{
name: "messages",
path: "locales/{locale}",
include: [fixture("collect/componentA"), fixture("collect/componentB.js")],
include: [
fixture("collect/componentA"),
fixture("collect/componentB.js"),
],
exclude: [],
},
mockConfig()
)

const messages = await catalog.collect({
...defaultMakeOptions,
files: [fixture("collect/componentA")]
files: [fixture("collect/componentA")],
})
expect(messages).toMatchSnapshot()
})
Expand Down Expand Up @@ -718,6 +721,34 @@ describe("getCatalogs", function () {
])
})

it("should expand {name} multiple times in path", function () {
mockFs({
componentA: {
"index.js": mockFs.file(),
},
})

const config = mockConfig({
catalogs: [
{
path: "{name}/locales/{locale}/{name}_messages_{locale}",
include: ["./{name}/"],
},
],
})
expect(getCatalogs(config)).toEqual([
new Catalog(
{
name: "componentA",
path: "componentA/locales/{locale}/componentA_messages_{locale}",
include: ["componentA/"],
exclude: [],
},
config
),
])
})

it("shouldn't expand {name} for ignored directories", function () {
mockFs({
componentA: {
Expand Down Expand Up @@ -813,6 +844,23 @@ describe("getCatalogForFile", function () {
expect(getCatalogForFile("./xyz/en.po", catalogs)).toBeNull()
})

it("should return matching catalog and locale if {locale} is present multiple times in path", function () {
const catalog = new Catalog(
{
name: null,
path: "./src/locales/{locale}/messages_{locale}",
include: ["./src/"],
},
mockConfig({ format: "po" })
)
const catalogs = [catalog]

expect(getCatalogForFile("./src/locales/en/messages_en.po", catalogs)).toEqual({
locale: "en",
catalog,
})
})

it("should return matching catalog and locale", function () {
const catalog = new Catalog(
{
Expand Down Expand Up @@ -934,12 +982,12 @@ describe("normalizeRelativePath", function () {
)
})

it("directories without ending slash are correctly treaten as dirs", function() {
it("directories without ending slash are correctly treaten as dirs", function () {
mockFs({
componentA: {
"index.js": mockFs.file(),
},
"componentB": mockFs.file(),
componentB: mockFs.file(),
})
// checked correctly that is a dir, cuz added that ending slash
expect(normalizeRelativePath("./componentA")).toEqual("componentA/")
Expand Down Expand Up @@ -1029,23 +1077,26 @@ describe("writeCompiled", function () {
name: "messages",
path: path.join(localeDir, "{locale}", "messages"),
include: [],
exclude: []
exclude: [],
},
mockConfig()
)

it.each([
{namespace: "es", extension: /\.mjs$/},
{namespace: "ts", extension: /\.ts$/},
{namespace: undefined, extension: /\.js$/},
{namespace: 'cjs', extension: /\.js$/},
{namespace: 'window.test', extension: /\.js$/},
{namespace: 'global.test', extension: /\.js$/}
])('Should save namespace $namespace in $extension extension', ({namespace, extension}) => {
const compiledCatalog = createCompiledCatalog("en", {}, {namespace})
// Test that the file extension of the compiled catalog is `.mjs`
expect(catalog.writeCompiled("en", compiledCatalog, namespace)).toMatch(
extension
)
})
{ namespace: "es", extension: /\.mjs$/ },
{ namespace: "ts", extension: /\.ts$/ },
{ namespace: undefined, extension: /\.js$/ },
{ namespace: "cjs", extension: /\.js$/ },
{ namespace: "window.test", extension: /\.js$/ },
{ namespace: "global.test", extension: /\.js$/ },
])(
"Should save namespace $namespace in $extension extension",
({ namespace, extension }) => {
const compiledCatalog = createCompiledCatalog("en", {}, { namespace })
// Test that the file extension of the compiled catalog is `.mjs`
expect(catalog.writeCompiled("en", compiledCatalog, namespace)).toMatch(
extension
)
}
)
})
32 changes: 19 additions & 13 deletions packages/cli/src/api/catalog.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ import { CliExtractTemplateOptions } from "../lingui-extract-template"
import { CompiledCatalogNamespace } from "./compile"

const NAME = "{name}"
const NAME_REPLACE_RE = /{name}/g
const LOCALE = "{locale}"
const LOCALE_REPLACE_RE = /{locale}/g
const LOCALE_SUFFIX_RE = /\{locale\}.*$/
const PATHSEP = "/" // force posix everywhere

Expand Down Expand Up @@ -264,10 +266,9 @@ export class Catalog {
const catalogs = this.readAll()
const template = this.readTemplate() || {}


return R.mapObjIndexed(
(_value, key) => this.getTranslation(catalogs, locale, key, options),
{ ...template, ...catalogs[locale] },
{ ...template, ...catalogs[locale] }
)
}

Expand Down Expand Up @@ -332,7 +333,8 @@ export class Catalog {
// We search in fallbackLocales as dependent of each locale
getMultipleFallbacks(locale) ||
// Get translation in fallbackLocales.default (if any)
(fallbackLocales?.default && getTranslation(fallbackLocales.default as string)) ||
(fallbackLocales?.default &&
getTranslation(fallbackLocales.default as string)) ||
// Get message default
catalog[key]?.defaults ||
// If sourceLocale is either target locale of fallback one, use key
Expand All @@ -348,7 +350,8 @@ export class Catalog {

write(locale: string, messages: CatalogType) {
const filename =
this.path.replace(LOCALE, locale) + this.format.catalogExtension
this.path.replace(LOCALE_REPLACE_RE, locale) +
this.format.catalogExtension

const created = !fs.existsSync(filename)
const basedir = path.dirname(filename)
Expand Down Expand Up @@ -390,7 +393,7 @@ export class Catalog {
ext = "js"
}

const filename = `${this.path.replace(LOCALE, locale)}.${ext}`
const filename = `${this.path.replace(LOCALE_REPLACE_RE, locale)}.${ext}`

const basedir = path.dirname(filename)
if (!fs.existsSync(basedir)) {
Expand All @@ -402,7 +405,8 @@ export class Catalog {

read(locale: string) {
const filename =
this.path.replace(LOCALE, locale) + this.format.catalogExtension
this.path.replace(LOCALE_REPLACE_RE, locale) +
this.format.catalogExtension

if (!fs.existsSync(filename)) return null
return this.format.read(filename)
Expand Down Expand Up @@ -477,7 +481,7 @@ export function getCatalogs(config: LinguiConfig): Catalog[] {
const correctPath = catalog.path.slice(0, -1)
const examplePath =
correctPath.replace(
LOCALE,
LOCALE_REPLACE_RE,
// Show example using one of configured locales (if any)
(config.locales || [])[0] || "en"
) + extension
Expand Down Expand Up @@ -526,7 +530,7 @@ export function getCatalogs(config: LinguiConfig): Catalog[] {
return
}

const patterns = include.map((path) => path.replace(NAME, "*"))
const patterns = include.map((path) => path.replace(NAME_REPLACE_RE, "*"))
const candidates = glob.sync(
patterns.length > 1 ? `{${patterns.join(",")}}` : patterns[0],
{
Expand All @@ -541,9 +545,11 @@ export function getCatalogs(config: LinguiConfig): Catalog[] {
new Catalog(
{
name,
path: normalizeRelativePath(catalog.path.replace(NAME, name)),
include: include.map((path) => path.replace(NAME, name)),
exclude: exclude.map((path) => path.replace(NAME, name)),
path: normalizeRelativePath(
catalog.path.replace(NAME_REPLACE_RE, name)
),
include: include.map((path) => path.replace(NAME_REPLACE_RE, name)),
exclude: exclude.map((path) => path.replace(NAME_REPLACE_RE, name)),
},
config
)
Expand All @@ -557,7 +563,7 @@ export function getCatalogs(config: LinguiConfig): Catalog[] {
export function getCatalogForFile(file: string, catalogs: Array<Catalog>) {
for (const catalog of catalogs) {
const catalogFile = `${catalog.path}${catalog.format.catalogExtension}`
const catalogGlob = catalogFile.replace(LOCALE, "*")
const catalogGlob = catalogFile.replace(LOCALE_REPLACE_RE, "*")
const match = micromatch.capture(
normalizeRelativePath(path.relative(catalog.config.rootDir, catalogGlob)),
normalizeRelativePath(file)
Expand All @@ -584,7 +590,7 @@ export function getCatalogForMerge(config: LinguiConfig) {
const correctPath = catalogConfig.catalogsMergePath.slice(0, -1)
const examplePath =
correctPath.replace(
LOCALE,
LOCALE_REPLACE_RE,
// Show example using one of configured locales (if any)
(config.locales || [])[0] || "en"
) + extension
Expand Down
7 changes: 2 additions & 5 deletions packages/cli/src/lingui-compile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,6 @@ if (require.main === module) {

// Check if Watch Mode is enabled
if (program.watch) {
const NAME = "{name}"
const LOCALE = "{locale}"

console.info(chalk.bold("Initializing Watch Mode..."))

const catalogs = getCatalogs(config)
Expand All @@ -194,8 +191,8 @@ if (require.main === module) {
catalogs.forEach((catalog) => {
paths.push(
`${catalog.path
.replace(LOCALE, locale)
.replace(NAME, "*")}${catalogExtension}`
.replace(/{locale}/g, locale)
.replace(/{name}/g, "*")}${catalogExtension}`
)
})
})
Expand Down
17 changes: 10 additions & 7 deletions packages/cli/src/services/translationIO.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,6 @@ function createPoItemFromSegment(segment) {
}

function saveSegmentsToTargetPos(config, paths, segmentsPerLocale) {
const NAME = "{name}"
const LOCALE = "{locale}"

Object.keys(segmentsPerLocale).forEach((targetLocale) => {
// Remove existing target POs and JS for this target locale
paths[targetLocale].forEach((path) => {
Expand All @@ -201,7 +198,12 @@ function saveSegmentsToTargetPos(config, paths, segmentsPerLocale) {
})

// Find target path (ignoring {name})
const localePath = "".concat(config.catalogs[0].path.replace(LOCALE, targetLocale).replace(NAME, ''), ".po")
const localePath = "".concat(
config.catalogs[0].path
.replace(/{locale}/g, targetLocale)
.replace(/{name}/g, ""),
".po"
)
const segments = segmentsPerLocale[targetLocale]

let po = new PO()
Expand Down Expand Up @@ -235,15 +237,16 @@ function saveSegmentsToTargetPos(config, paths, segmentsPerLocale) {
}

function poPathsPerLocale(config) {
const NAME = "{name}"
const LOCALE = "{locale}"
const paths = []

config.locales.forEach((locale) => {
paths[locale] = []

config.catalogs.forEach((catalog) => {
const path = "".concat(catalog.path.replace(LOCALE, locale).replace(NAME, "*"), ".po")
const path = "".concat(
catalog.path.replace(/{locale}/g, locale).replace(/{name}/g, "*"),
".po"
)

// If {name} is present (replaced by *), list all the existing POs
if (path.includes('*')) {
Expand Down