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

Defere loading to ensure translations work #187

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

martin-ro
Copy link

I think I discovered a bit of a funny bug: The translation of components in blocks is actually only working because of the package name starting with "z": Z3d0X/...

The translation service is not available at the time the blocks are registered so translations will be completely ignored.

This is easily demonstrated in this repo: https://github.com/martin-ro/fabricator-translation

I've cloned z3d0x/filament-fabricator into the packages directory and changed the name to "aaaz3d0x/filament-fabricator". You'll see that a CuratorPicker for example is not translated anymore.

This really is an edge case and only affects packages that are alphabetically after "z3d0x" loaded (that's how I discovered it).

However, I think it's still a bug worth fixing. If you ever move the package into another namespace it will break for everyone depending on naming.

This PR implements https://laravel.com/docs/11.x/providers#deferred-providers:

... you may choose to defer its registration until one of the registered bindings is actually needed Deferring the loading of such a provider will improve the performance of your application, since it is not loaded from the filesystem on every request.
... Then, only when you attempt to resolve one of these services does Laravel load the service provider.

Personally, I only use the blocks and nothing else. I don't know how that would interfere with the rest of the package.
What do you think @Z3d0X ?

Best,

Martin

Copy link

what-the-diff bot commented Oct 27, 2024

PR Summary

  • Adoption of DeferrableProvider Interface in FilamentFabricatorServiceProvider
    The code now includes the DeferrableProvider interface to the FilamentFabricatorServiceProvider. This change means that the service provider loading can now be delayed until needed, potentially improving system performance by reducing unnecessary loads at startup.

  • Introduction of a 'provides' method
    A new provides() method has been added, which is designed to return the ID of the FilamentFabricatorManager. This provides a means of easily identifying and retrieving the specific manager when it is required, enhancing maintainability and speeding up the lookup process.

@martin-ro
Copy link
Author

Now that I think about it, I belive it would be better to do something like:

 public function bootingPackage(): void
    {
        $this->app->afterResolving('translator', function () {
            $this->registerComponentsFromDirectory(
                PageBlock::class,
                config('filament-fabricator.page-blocks.register'),
                config('filament-fabricator.page-blocks.path'),
                config('filament-fabricator.page-blocks.namespace')
            );
        });

        $this->registerComponentsFromDirectory(
            Layout::class,
            config('filament-fabricator.layouts.register'),
            config('filament-fabricator.layouts.path'),
            config('filament-fabricator.layouts.namespace')
        );
    }
    ```

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