nWidart / laravel-modules

Module Management In Laravel
https://docs.laravelmodules.com
MIT License
5.5k stars 954 forks source link

Streamline Provider Registrations with a Single File #1879

Closed alissn closed 1 month ago

alissn commented 3 months ago

Hi,

In this pull request, I have updated the provider registrations to use a single file.

Changes

Currently, a file is created for each module in the bootstrap/cache directory. With this update, a manifest class is created to register all providers of modules in one file. I haven't deleted the old logic yet; if @dcblogdev and the community agree with this pull request, we can remove the old logic afterward.

Next Steps

The next step is to simplify the module providers further by registering migrations, languages, components, facades, etc., inside the package to improve the package's load performance. This addresses issue #1745.

Added Commands

  1. module:clear-compiled

    • Deletes the compiled modules.php file.
      php artisan module:clear-compiled
  2. module:discover

    • Recreates the compiled modules.php file.
      php artisan module:discover

Before

➜  cache git:(master) ✗ tree
.
├── module0_module.php
├── module100_module.php
... // other files
├── module97_module.php
├── module98_module.php
├── module99_module.php
├── module9_module.php
├── packages.php
└── services.php

Each file contains:

<?php return array (
  'providers' => 
  array (
    0 => 'Modules\\Module4\\Providers\\Module4ServiceProvider',
  ),
  'eager' => 
  array (
    0 => 'Modules\\Module4\\Providers\\Module4ServiceProvider',
  ),
  'deferred' => 
  array (
  ),
);

After

➜  cache git:(master) ✗ tree
.
├── modules.php
├── packages.php
└── services.php

modules.php content:

<?php return [
    'providers' => [
        0   => 'Modules\\Module0\\Providers\\Module0ServiceProvider',
        1   => 'Modules\\Module0\\Providers\\EventServiceProvider',
        ... // other providers
        152 => 'Modules\\Module99\\Providers\\Module99ServiceProvider',
    ],
    'eager' => [
        0   => 'Modules\\Module0\\Providers\\Module0ServiceProvider',
        1   => 'Modules\\Module0\\Providers\\EventServiceProvider',
        ...  // other providers
        152 => 'Modules\\Module99\\Providers\\Module99ServiceProvider',
    ],
    'deferred' => [],
];

If anyone has ideas to improve this pull request, please let me know.

solomon-ochepa commented 3 months ago

Wow, this is nice!

However, for flexibility and future integration purposes, I'll recommend the following design?

<?php

return [
    'module0' => [
        'providers' => [
            'Modules\\Module0\\App\\Providers\\Module0ServiceProvider',
            'Modules\\Module0\\App\\Providers\\EventServiceProvider',
        ],
        'eager' => [
            'Modules\\Module0\\App\\Providers\\Module0ServiceProvider',
            'Modules\\Module0\\App\\Providers\\EventServiceProvider',
        ],
        'deferred' => [],
    ],
    'module1' => [
        'providers' => [
            'Modules\\Module1\\App\\Providers\\Module1ServiceProvider',
            'Modules\\Module1\\App\\Providers\\EventServiceProvider',
        ],
        'eager' => [
            'Modules\\Module1\\App\\Providers\\Module1ServiceProvider',
            'Modules\\Module1\\App\\Providers\\EventServiceProvider',
        ],
        'deferred' => [],
    ],
];

Note: The emphasis here is grouping each module into a different array - each module will be isolated and easy to manipulate/manage.

alissn commented 3 months ago

@solomon-ochepa

The module.php file is registered by \Illuminate\Foundation\ProviderRepository and does not support your recommended design !!

solomon-ochepa commented 3 months ago

@solomon-ochepa

The module.php file is registered by \Illuminate\Foundation\ProviderRepository and does not support your recommended design !!

Oh, it's okay.

dcblogdev commented 3 months ago

This looks good, would this improve the performance at all or a foundation for the next step?

Also could you add tests for the commands.

alissn commented 3 months ago

Hi, @dcblogdev,

Thanks for your review.

I'll add some tests for the new command. I want to ensure this pull request is good for you, so I'll create the tests and make other improvements. I'm also reviewing the package logic and making some improvements to enhance loading speed. This PR, along with #1879, lays the foundation for these next steps, aiming to simplify generated files and improve overall performance.

I made this pull request a draft to create tests.

alissn commented 1 month ago

Hi @dcblogdev,

I've completed this merge request, and it's ready for review.

Some details: