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

#1097 Remove i18nMap and switch to regex based invalid key check #1130

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

andrewtavis
Copy link
Member

Contributor checklist


Description

This PR addresses development and performance issues introduced by the i18nMap object by replacing it with a method that's more suitable for the project. Note that the i18n-check project has gotten this change in parallel.

Issues with the i18nMap include:

  • It's something that would need to be updated, which is confusing for developers
  • In loading in i18nMap to most frontend files, we will doubtless experience some performance issues in that we'll have a JSON object with 1,000s of keys being loaded into most components and pages

The solution to the above issues is twofold:

  • Switch over the check of invalid i18n keys (those that we've incorrectly typed, etc) from a type check to a regex check
  • In order to allow for a regex check of all i18n keys, they should all be prepended with i18n.
    • This allows us to find them with r"\"i18n\.[_\S\.]+?\"" and similar patterns

With this the i18n process for the project is set. Further updates to this will come after i18n-check has been released via a version update over here :)

Related issue

Copy link

netlify bot commented Feb 18, 2025

Deploy Preview for activist-org ready!

Name Link
🔨 Latest commit 4dc1fce
🔍 Latest deploy log https://app.netlify.com/sites/activist-org/deploys/67b3e32e48f22c0008540cd0
😎 Deploy Preview https://deploy-preview-1130--activist-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

Thank you for the pull request! ❤️

The activist team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Development rooms once you're in. Also consider attending our bi-weekly Saturday developer syncs! It'd be great to meet you 😊

Copy link
Contributor

github-actions bot commented Feb 18, 2025

Maintainer Checklist

The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

  • The TypeScript and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The Playwright end to end and Zap penetration tests have been ran and are passing (if necessary)

  • The changelog has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

@andrewtavis
Copy link
Member Author

CC @mattburnett-repo, @cquinn540, @aasimsyed:

Wanted to bring these changes to your all's attention. We can talk about this a bit more in the sync, but the i18nMap was a meh idea that ultimately lead to a better one 😊 If we're saying that i18n keys should be accessed via an object such that we'll have i18nMap.* for all i18n references, why not just prepend all strings with i18n. such that we can use regex to find them? This means that:

  • We don't need to update the map object as we're back to using the strings
  • We have a new regex check to see if any keys used in the project are not included in the en-US.json file
  • We don't need to import i18nMap into most files in the frontend and doubtless deal with negative drawbacks of dev experience and performance from loading a large object everywhere

@OmarAI2003: The changes to the checks have already been made in i18n-check, so you're still free to work on activist-org/i18n-check#2 😊

@andrewtavis
Copy link
Member Author

andrewtavis commented Feb 18, 2025

The above changes came post checking our strategies at the Vue.js Berlin Meetup :)

@andrewtavis
Copy link
Member Author

Bringing this in so that we can finalize this quick pivot 😊

@andrewtavis andrewtavis merged commit 369f97f into main Feb 18, 2025
9 checks passed
@andrewtavis andrewtavis deleted the 1097-i18n-check-invalid-keys branch February 18, 2025 01:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant