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

[11.x] Optimize loadTranslationsFrom function for simplicity and clarity #54407

Merged
merged 12 commits into from
Jan 30, 2025

Conversation

selcukcukur
Copy link
Contributor

Current Behavior

When developing packages or performing specific translation loading operations, there are two methods available for loading translations: loadJsonTranslationsFrom and loadTranslationsFrom. Currently, it is not possible to register translations without a namespace using the loadTranslationsFrom method.

class FooServiceProvider extends ServiceProvider
{
     public function register()
     {
          $this->loadTranslationsFrom(__DIR__.'/../path/to', 'namespace');
          // OR
          $this->loadJsonTranslationsFrom(__DIR__.'/../path/to');
     }
}

Issue

When using the loadTranslationsFrom method to register a translation, the only way to use it with the __(), trans(), or trans_choice() methods is by specifying the namespace, like namespace::file.foo.bar. This often results in repetitive and long names when developing packages, making the code cumbersome and less readable.

Example :

// To call a translation registered this way, you would need to use:
$this->loadTranslationsFrom(__DIR__.'/../path/to', 'namespace');

// While this is not a major issue in general, it becomes problematic when the file name and namespace are the same or when the line lengths increase due to specific requirements, leading to repetitive and cluttered code.
trans('namespace::file.foo.bar')
// OR
trans('namespace::namespace.foo.bar')

Although this issue can be resolved using loadJsonTranslationsFrom(), it may cause specific problems when multiple packages are used, and JSON translations do not offer the same functionality as the loadTranslationsFrom() method.

Proposed Change

We propose adding an addPath method to the Translator and FileLoader classes and making the $namespace parameter in the ServiceProvider class's loadTranslationsFrom method default to null. When a null value is provided, the directory should be included in the translation loader without needing a namespace.

This change will not break backward compatibility and will allow developers to load external translation directories without a namespace, preserving the original behavior.

class FooServiceProvider extends ServiceProvider
{
    public function register()
    {
        // New (Usage: trans('file.foo.bar'))
        $this->loadTranslationsFrom(__DIR__.'/../path/to');
        // OR (Usage: trans('namespace::file.foo.bar'))
        $this->loadTranslationsFrom(__DIR__.'/../path/to', 'namespace');
        // OR (Usage: trans('Foo bar'))
        $this->loadJsonTranslationsFrom(__DIR__.'/../path/to');
    }
}

Backward Compatibility

This pull request does not create any backwards incompatibility. Added some specific tests to ensure it won't work.

@taylorotwell taylorotwell merged commit 01571b3 into laravel:11.x Jan 30, 2025
38 checks passed
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