Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Using JSON5 for en.json #1815

Closed
2 tasks
dhruvkb opened this issue Sep 15, 2022 · 2 comments · Fixed by #1949
Closed
2 tasks

Using JSON5 for en.json #1815

dhruvkb opened this issue Sep 15, 2022 · 2 comments · Fixed by #1949
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature 🟩 priority: low Low priority and doesn't need to be rushed

Comments

@dhruvkb
Copy link
Member

dhruvkb commented Sep 15, 2022

Due date: 2022-09-29 (a fortnight)

Assigned reviewers

Tagging @WordPress/openverse-frontend, please feel free to add your name below if you have some feedback (good or bad) or ideas.

  • TBD
  • TBD

Description

We use JSON to write the main i18n locale file: en.json. This file is completely written by hand and maintained manually, for which JSON is a pretty poor format. This might come across as a hot-take so let me elaborate.

Comments

My main complaint with using JSON for en.json is lack of comments. It's very hard to describe context for any i18n key in the en.json file. For a file that's written by and for people, that's a huge miss. There is duplication in the file, many keys are written deeply nested and then used outside of where they're defined (happens during refactors when code is moved around) and then there's no way to know if an i18n key is deprecated/superseded.

Comments wouldn't solve the problems above but they would be very helpful. That's why POT files (like openverse.pot) have comments (the #: before each msg)! And since our en.json file doesn't have comments, our POT comments end up being just .vue references. I believe we are under-utilising a powerful tool for communicating intent and context with the translators.

Syntax

I also have several personal gripes with the JSON syntax but that's secondary.

  • lack of trailing commas
  • too many quotes, especially double quotes

Why JSON5?

JSON5 is a superset of JSON which has many advantages over plain JSON. I'll let their webpage explain what those are. Switching to JSON5 is as easy as a rename (superset of JSON) and we can adopt changes incrementally.

Migration path

Feel free to chime in with alternatives.

  • Using JSON5 for en.json (→ en.json5) and moving it out of src/locales/.
  • Writing a script that parses en.json5 and converts it to en.json.
    • Using the new JSON5 when it is being synced with GlotPress (to pass comments to translators).
    • Writing the plain JSON into src/locales/ similar to other locales.
  • Ignoring the plain en.json file in src/locales/.
    • This makes the special treatment we give to en.json5 much clearer (placed separately) and makes the en.json file equivalent to any other locale file (except generated locally instead of downloaded).
      # .gitignore
      src/locales/*.json
      !src/locales/en.json

Alternatives

YAML, although almost universally reviled, is imo a great format. But moving to YAML is not the right move.

  • YAML is not liked by a lot of developers (understandably). JSON on the other is much beloved, especially in the JS community.
  • YAML requires a file rewrite (which can be automated, but why do it in the first place) when JSON is already valid JSON5.
  • JSON5 is already being used in the codebase, for e2e tapes (that's precedent).

References

@dhruvkb dhruvkb added 🟩 priority: low Low priority and doesn't need to be rushed ✨ goal: improvement Improvement to an existing user-facing feature 🤖 aspect: dx Concerns developers' experience with the codebase 💬 talk: discussion Open for discussions and feedback labels Sep 15, 2022
@krysal
Copy link
Member

krysal commented Sep 15, 2022

there's no way to know if an i18n key is deprecated/superseded.

There is indeed a vue-i18n rule to warn about unused keys. I agree would be great to add it and to have comments in JSONs 😃

@zackkrida
Copy link
Member

I support this, especially if we can find a way to get the json5 comments into the pot file and subsequently into glotpress.

@dhruvkb dhruvkb mentioned this issue Oct 30, 2022
7 tasks
@dhruvkb dhruvkb removed the 💬 talk: discussion Open for discussions and feedback label Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🤖 aspect: dx Concerns developers' experience with the codebase ✨ goal: improvement Improvement to an existing user-facing feature 🟩 priority: low Low priority and doesn't need to be rushed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants