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

Add an isolating router #541

Merged
merged 1 commit into from
Dec 16, 2023
Merged

Conversation

michaelbaudino
Copy link
Contributor

This new router considers each source YAML file independant from each other, thus allowing each of them to include similar keys.

Here is an overview of how it works:

  • when reading YAML files (as defined by patterns in the data.read configuration, like for other routers), it does not load them all in a common global space, but rather creates a top-level node for YAML file and then loads its content inside this node.
  • when routing a given forest, it actually iterates on all top-level keys (which are source file paths) then yields the given block for each top-level key, passing also its subtree.

Note that the top-level key (which is the source file path) had to be enclosed in a character in order to prevent considering its dots (.) as a nesting level when de-flattening them. A mechanism to do this already existed for {}, [] and () but all of those enclosers have a meaning in regular expressions. Thus, this commit extends this mechanism to support <> and uses them as encloser for the top-level key. It also has the advantage of making this special part of the flattened key easily distinguishable by a human eye.

For example, the following YAML source files:

  • app/components/movies_component.en.yml:

    en:
      title: Movies
  • app/components/games_component.en.yml

    en:
      title: Games

… result in the following forest once loaded in memory:

en:
  <app/components/movies_component.en.yml>:
    title: Movies
  <app/components/games_component.en.yml>:
    title: Games

… which in turn results in the following flattened keys:

en.<app/components/movies_component.en.yml>.title: Movies
en.<app/components/games_component.en.yml>.title: Games

… which can be sent to any translator and should come back as:

fr.<app/components/movies_component.en.yml>.title: Flims
fr.<app/components/games_component.en.yml>.title: Jeux

… finally, this router allows re-assigning each key to the appropriate target file, which gives:

  • app/components/movies_component.fr.yml:

    fr:
      title: Flims
  • app/components/games_component.fr.yml

    fr:
      title: Jeux

This implicitely adds support for ViewComponent (which suggests organizing YAML files in "sidecars" and which adds an implicit scope to keys in those files, based on their file path).

This new router considers each source YAML file independant from each other, thus allowing each of them to include similar keys.

Here is an overview of how it works:
* when reading YAML files (as defined by patterns in the `data.read` configuration, like for other routers), it does not load them all in a common global space, but rather creates a top-level node for YAML file and then loads its content inside this node.
* when routing a given forest, it actually iterates on all top-level keys (which are source file paths) then yields the given block for each top-level key, passing also its subtree.

Note that the top-level key (which is the source file path) had to be enclosed in a character in order to prevent considering its dots (`.`) as a nesting level when de-flattening them. A mechanism to do this already existed for `{}`, `[]` and `()` but all of those enclosers have a meaning in regular expressions. Thus, this commit extends this mechanism to support `<>` and uses them as encloser for the top-level key. It also has the advantage of making this special part of the flattened key easily distinguishable by a human eye.

For example, the following YAML source files:

* `app/components/movies_component.en.yml`:

   ```yaml
   en:
     title: Movies
   ```

* `app/components/games_component.en.yml`

   ```yaml
   en:
     title: Games
   ```

… result in the following forest once loaded in memory:
```yaml
en:
  <app/components/movies_component.en.yml>:
    title: Movies
  <app/components/games_component.en.yml>:
    title: Games
```

… which in turn results in the following flattened keys:
```
en.<app/components/movies_component.en.yml>.title: Movies
en.<app/components/games_component.en.yml>.title: Games
```

… which can be sent to any translator and should come back as:
```
fr.<app/components/movies_component.en.yml>.title: Flims
fr.<app/components/games_component.en.yml>.title: Jeux
```

… finally, this router allows re-assigning each key to the appropriate target file, which gives:

* `app/components/movies_component.fr.yml`:

   ```yaml
   fr:
     title: Flims
   ```

* `app/components/games_component.fr.yml`

   ```yaml
   fr:
     title: Jeux
   ```

This implicitely adds support for ViewComponent (which suggests organizing YAML files in "sidecars" and which adds an implicit scope to keys in those files, based on their file path).
@glebm glebm merged commit 30b7703 into glebm:main Dec 16, 2023
6 checks passed
@glebm
Copy link
Owner

glebm commented Dec 16, 2023

I'm guessing you have a plan to address the missing keys limitation :)

@michaelbaudino michaelbaudino deleted the add-isolating-router branch December 16, 2023 10:04
@michaelbaudino
Copy link
Contributor Author

Well, to be honest: not really 😭

If I understand the problem correctly, and with my use case in mind (which is ViewComponent, but we probably want a generic-enough solution, that does not tightly couple i18n-tasks with ViewComponent), I see multiple problems described below (with some potential solutions).

1. Translations may live in multiple files

Problem

For a given ViewComponent (let's use MoviesComponent as an example), its i18n translations may live in multiple files (all of them are valid and considered part of the component sidecar):

  • app/components/movies_component/movies_component.en.yml (subdirectory method)
  • app/components/movies_component.en.yml (locale-specific sibling file method)
  • app/components/movies_component.yml (sibling file method containing multiple locales, not supported by current isolating_router implementation)

Thus, checking if a given translation exists (e.g. when encountering t(".title") in a component) in a tree of translations namespaced by file name is not straightforward.

Solution 1: try all files

Look into all possible i18n files:

  • app/components/movies_component.en.yml
  • app/components/movies_component/movies_component.en.yml
  • app/components/movies_component.en.yaml
  • app/components/movies_component/movies_component.en.yaml
  • app/components/movies_component.en.json
  • app/components/movies_component/movies_component.en.json

Potentially also in currently unsupported files:

  • app/components/movies_component.yml
  • app/components/movies_component/movies_component.yml
  • app/components/movies_component.yaml
  • app/components/movies_component/movies_component.yaml
  • app/components/movies_component.json
  • app/components/movies_component/movies_component.json

It might be a bit tedious and should be configurable if we don't want to be tightly coupled to ViewComponent. For example like that:

data:
  # regexp that defines all files sharing a common implicit i18n scope
  implicit_scope_selector: 'app/components/(%{id}/)?/%{id}(.%{locale})?.{yml,yaml,json}'

Solution 2: namespace with implicit scope rather than original file path

When reading i18n files to load all translations in memory, we could namespace translations in their implicit scope (see problem 2 below to see how this implicit scope can be computed 👇) rather than in their file path (current behavior of isolating_router).

Thus, the following YAML file:

# app/components/collections/movies_component.en.yml
en:
 title: Movies

… could be loaded in memory as the following tree:

en:
  <collections.movies_component>:
    title: Movies

… rather than the current behaviour that loads the same file as follows:

en:
  <app/components/collections/movies_component.en.yml>:
    title: Movies

1b. Routing a translation to a file must ensure consistency with source file

Problem

If we choose solution 2 above (i.e. namespacing with implicit scope rather than with original file name), routing (i.e. finding out in which file must be written a translation based on its flat key) must now determine in which of the multiple files the translation (in target locale) should be written, since we don't know the source file it originally comes from anymore.

For example, given the following source file:

# app/components/movies_component/movies_component.en.yml
en:
  title: Movies

… it would be stored in memory like that:

en:
  <collections.movies_component>:
    title: Movies

The router should write translations in target locales (e.g. French) to app/components/movies_component/movies_component.fr.yml rather than app/components/movies_component.fr.yml, despite both files being actually valid options.

Solution 1: namespace with both implicit scope and original file path

We could namespace in memory with both the source file path and the implicit scope, e.g. separated with a colon (:) or a pipe (|):

en:
  <collections.movies_component:app/components/collections/movies_component/movies_component.en.yml>:
    title: Movies

This would allow us to parse the key:

  • the "i18n scope" part would allow us properly detecting if a given translation is used (still solving problem 1 above ☝️).
  • the "file path" part would allow us to know where to write a given translation.

Solution 2: detect from which file a given flat key originates

During the routing phase, we could try to detect which file a given translation originally came from, by re-reading all source i18n files. This could lead us to choose the appropriate alternate file to write to.

I'm not sure how accurate this detection can be, but it could probably use the implicit_scope_selector configuration (described in solution 1 of problem 1 above ☝️):

data:
  # regexp that defines all files sharing a common implicit i18n scope
  implicit_scope_selector: 'app/components/(%{id}/)?/%{id}(.%{locale})?.{yml,yaml,json}'

2. Implicit scope logic not known

Problem

The implicit scope applied to each file is a logic known only by the third-party library (in my case: ViewComponent) and potentially changing in different library versions.

Thus, namespacing an i18n file content with it (as suggested in solution 2 of problem 1 above ☝️) or checking if a key exist when parsing a call to translate in a component or template file requires to generate this implicit scope.

Solution

Configure implicit scope logic as a chain of regular expression substitutions applied to the file name and that would produce the implicit i18n scope to be applied to translations in this file.

For example, current ViewComponent implicit scope could be generated using the following configuration:

data:
  # list of "pattern + substitution" couples passed to a chain of `gsub` calls applied to the file name
  implicit_scope_transformers:
    - ['{app/components/**/*}.{rb,en.ya?ml,erb,html,slim}', '\1.\2']
    - ['{*}_component/{*}_component.', '\1_component.']
    - ['^/', '']
    - ['/_?', '.']

Warning

This works with ViewComponent logic which involves a chain of regular expression substitutions, but other implicit scope generation logics may not be defined using only regular expressions!

For example, a logic that would force or swap character case could not be implemented using gsub only (since Ruby regular expressions do not support \U and \L substitution tokens, like Perl does…), neither would a logic with even more complex manipulations.

3. Missing keys exhaustiveness

Problem

ViewComponent supports both translations with implicit scope (t(".title")) and classic translations (t("my.global.translation")).

In the process of checking if a translation exists for any given call to translate parsed, we must be able to look into both namespaced translations (as loaded when the isolating_router is enabled) and classic (non-namespaced) translations (as loaded when other routers are enabled).

This is currently not possible because a single router directive is active for each i18n-tasks configuration.

Solution 1: use multiple routers in same configuration

In order to have both a namespaced (with either implicit scopes, file names or both 🤷) and non-namespaced trees loaded at the same time, we need to be able to configure different routers for different file paths, for example:

data:
  router:
    isolating_router:
      only:
        - app/components/**/*.%{locale}.yml
    conservative_router:
      only:
        - config/locales/**/*.%{locale}.yml

It would probably also require the routing mechanism to detect if a given flat key is a namespace of not (i.e. if it is enclosed by <>), to eventually delegate to the appropriate router.


I'd love to have other people opinions.

Am I seeing problems that do not exist?

Am I over-complicating all this?

Or would it take a lot of changes to support missing keys detection properly with isolating_router?

@michaelbaudino
Copy link
Contributor Author

I realize the length of my latest comment may be a bit excessive, I'm sorry 😬

But if you had an idea that is straightforward to implement, feel free to share it and I'd be happy to work on it.

@glebm
Copy link
Owner

glebm commented Dec 20, 2023

Perhaps something along the lines of how we handle mailers?

Here, we modify the key in the scanner based on the filename

# @param normalized_path [String] path/relative/to/a/root
# @param calling_method [#call, Symbol, String, false, nil]
def prefix(normalized_path, calling_method: nil)
file_key = normalized_path.gsub(%r{(\.[^/]+)*$}, '').tr(File::SEPARATOR, DOT)
calling_method = calling_method.call if calling_method.respond_to?(:call)
if calling_method&.present?
# Relative keys in mailers have a `_mailer` infix, but relative keys in controllers do not have one:
"#{file_key.sub(/_controller$/, '')}.#{calling_method}"
else
# Remove _ prefix from partials
file_key.gsub(/\._/, DOT)
end

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.

2 participants