Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for serializing and deserializing locale-specific versions of entries in Contentstack. The implementation allows entries to be stored either as single files (for backward compatibility) or as multiple locale-specific files with the naming pattern entry.locale.yaml.
Changes:
- Added new API functions to export and fetch locale-specific entry data
- Modified entry import/export logic to handle multiple locale versions
- Updated filesystem indexing to recognize both single-locale and multi-locale file patterns
- Implemented backward compatibility by supporting simple filename format for single-locale entries
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
cli/src/cs/entries/exportEntryLocale.ts |
New function to export entry data for a specific locale |
cli/src/cs/entries/getEntryLocales.ts |
New function to fetch available locales for an entry |
cli/src/cs/entries/getEntryLocales.test.ts |
Tests for locale info structure and filename patterns |
cli/src/schema/entries/lib/loadEntryLocales.ts |
New function to load all locale versions of an entry from filesystem |
cli/src/schema/entries/lib/loadEntryLocales.test.ts |
Basic tests for loadEntryLocales function |
cli/src/schema/entries/toFilesystem.ts |
Modified export logic to write locale-specific files |
cli/src/schema/entries/toContentstack.ts |
Modified import logic to handle locale-specific files |
cli/src/schema/entries/lib/buildCreator.ts |
Updated entry creation to import all locale versions |
cli/src/schema/entries/indexAllFsEntries.ts |
Updated indexing to recognize multi-locale file patterns |
cli/src/cs/entries/import.ts |
Added locale parameter to import function |
cli/src/cs/entries/lib/importCreate.ts |
Added locale parameter to API call |
cli/src/cs/entries/lib/importOverwrite.ts |
Added locale parameter to API call |
test/integration/workflow/lib/loadEntry.ts |
Updated test helper to support locale-specific file loading |
altearius
left a comment
altearius
left a comment
There was a problem hiding this comment.
Partial review only, I looked over it, but I have some questions about the functionality before I do a full review.
In the file system, what is the intended structure for handling locale-specific versions of entries?
How will this handle backwards compatibility with projects that have already serialized content?
How does this handle serialization of non-localizable fields on locale-specific versions of entries?
How does this handle references between entries / assets?
What happens during a push if we attempt to push a localized version of an entry for which the fallback entry does not yet exist? Is it possible for such a situation to occur?
The management API has endpoints related to localization: Localize an Entry, Update a Localized Entry, Unlocalize an Entry. Are these being used? If they aren't being used, can we add some documentation (under /doc or /doc/lessons-learned) that describes why not?
My reading of the documentation is that, when working with localized entries, the order in which we manage locales is important. We should work with the default language first, then work with every language that uses the default language as a fallback, then work with languages that use those as fallbacks, and so on. Is that being done? If not, what happens when this order is violated?
What happens when we delete an entry that has localized versions? Are the localized versions automatically deleted?
How are localized assets handled?
| * const pattern = new RegExp(`^${escapeRegex(filename)}\\.yaml$`, 'u'); | ||
| * // Creates pattern: /^My\.Entry\.Title\.yaml$/u | ||
| */ | ||
| export default function escapeRegex(str: string): string { |
There was a problem hiding this comment.
This should already exist, is RegExp.escape not working?
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/escape
Also, why is this needed at all?
There was a problem hiding this comment.
Regarding RegExp.escape:
RegExp.escape is a Stage 3 TC39 proposal but is not yet available in Node.js 22. It's not part of the JavaScript standard yet. Once it reaches Stage 4 and is implemented in Node.js, you could replace this custom implementation.
Why is this needed?
The function is used in 3 locations to escape filenames when creating regex patterns for matching entry locale files:
loadEntryLocales.ts:73 - Creates pattern like /^My.Entry.Title.([^.]+).yaml$/ to match locale variants
toFilesystem.ts:135 - Creates pattern to match entry files with any locale
loadEntry.ts:24 - Used in tests
Without escaping, a filename like My.Entry.Title would have . treated as "any character" in the regex instead of literal dots, causing incorrect matches.
Recommendation:
Keep the custom implementation for now since RegExp.escape isn't available yet. When Node.js supports it (likely Node.js 24+), you can replace this with the native implementation.
| // If simple format doesn't exist, try finding locale-specific file | ||
| try { | ||
| const files = await readdir(entriesDir); | ||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-call -- escapeRegex is properly typed as (string) => string |
There was a problem hiding this comment.
This line generates a lint error for me:
$ yarn lint
/home/cnielsen/projects/beacon/test/integration/workflow/lib/loadEntry.ts
23:4 warning Unused eslint-disable directive (no problems were reported from '@typescript-eslint/no-unsafe-call')
✖ 1 problem (0 errors, 1 warning)
0 errors and 1 warning potentially fixable with the `--fix` option.CI/CD should be reporting this, and it isn't -- but that's an unrelated issue with CI/CD.
There was a problem hiding this comment.
I'm not seeing this pop up as an error locally when running yarn lint.
Added /doc/lessons-learned/multi-language-support.md to outline the answers to the questions. |
Added support for serialization/de-serialization of locale versions of entries