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

Jump to bottom

yarn merge-i18n script always unquotes the "title" key #2309

Open
alanorth opened this issue Jun 14, 2023 · 1 comment · May be fixed by #2317
Open

yarn merge-i18n script always unquotes the "title" key #2309

alanorth opened this issue Jun 14, 2023 · 1 comment · May be fixed by #2317
Labels
bug i18n / l10n Internationalisation and localisation, related to message catalogs low priority

Comments

@alanorth
Copy link
Contributor

alanorth commented Jun 14, 2023 •
edited
Loading

Describe the bug
The yarn merge-i18n script unquotes the title key, which results in an invalid json5 file (at least according to our eslint rules).

To Reproduce
Steps to reproduce the behavior:

  1. Run the yarn merge-i18n script against the default custom theme:
$ yarn merge-i18n -s src/themes/custom/assets/i18n
yarn run v1.22.19
$ ts-node --project ./tsconfig.ts-node.json scripts/merge-i18n-files.ts -s src/themes/custom/assets/i18n
Merging: /home/aorth/src/git/dspace-angular/src/assets/i18n/en.json5 with src/themes/custom/assets/i18n/en.json5
 ████████████████████████████████████████ 100% | ETA: 0s | 100/100
Done in 0.75s.
  1. Check the resulting src/assets/i18n/en.json5 and see the title key is unquoted:
diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5
index 480bf8834..7ffc7b61a 100644
--- a/src/assets/i18n/en.json5
+++ b/src/assets/i18n/en.json5
@@ -2786,7 +2786,7 @@
 
   "menu.section.browse_community_by_title": "By Title",
 
-  "menu.section.browse_global": "All of DSpace",
+  "menu.section.browse_global": "Browse DSpace",
 
   "menu.section.browse_global_by_author": "By Author",
 
@@ -3464,7 +3464,7 @@
 
   "repository.title": "DSpace Repository",
 
-  "repository.title.prefix": "DSpace Repository :: ",
+  "repository.title.prefix": "DSpace :: ",
 
   "resource-policies.add.button": "Add",
 
@@ -4696,7 +4696,7 @@
 
   "thumbnail.person.placeholder": "No Profile Picture Available",
 
-  "title": "DSpace",
+  title: "DSpace",
 
   "vocabulary-treeview.header": "Hierarchical tree view",

Expected behavior
According to our ESLint rules, json5 keys must be quoted, though it doesn't seem this is a requirement from the json5 spec.

This issue might be related: json5/json5#32
@alanorth alanorth added bug i18n / l10n Internationalisation and localisation, related to message catalogs low priority labels Jun 14, 2023
@alanorth alanorth linked a pull request Jun 16, 2023 that will close this issue
Open
8 tasks
@alanorth
Copy link
Contributor Author

Ah, of course, the json5 library will avoid quoting keys if possible because that is valid in JSON5. The title key is a single word with no special characters that need quoting so json5.stringify is being opportunistic and trying to save us a few bytes.

So the issue here is that our i18n assets are in JSON5 format for human readability, but our current ESLint rules are validating them as JSONC (JSON with comments). This would be partially solved if we switched our ESLint rules to use JSON5 (see #2317).

Having said that, I'm not sure if we actually need our i18n assets to be in JSON5, as it seems we are essentially only using comments, and the Angular frontend is using them in plain JSON format anyway (see src/ngx-translate-loaders/translate-server.loader.ts).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug i18n / l10n Internationalisation and localisation, related to message catalogs low priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update ESLint configuration for json5 files alanorth/dspace-angular
1 participant
@alanorth