search

userfrosting / UserFrosting

Modern PHP user login and management framework
https://www.userfrosting.com
Other
1.63k stars 368 forks source link

UserFrosting 5.0 Planning #1095

Closed lcharette closed 3 years ago

lcharette commented 4 years ago

Following my first attempt at moving into the UF 5.0 structure, I think we need to define a formal plan regarding the repo and class structure for the future version of UserFrosting.

We already know the main goals for v5 are :

  1. Move core, account and admin to their own repo and create a UF skeleton repo / template (#830), so sprinkles are managed by composer, easy to remove if not-needed and easier to test.
  2. Add vendor name to sprinkles, to avoid conflicts (#831).
  3. Have services providers and bakery commands be deliberately registered instead of auto-discovered

Creating a skeleton repo means all base code not in the core Sprinkle needs to be moved somewhere else, especially the sprinklemanager.

To better illustrate the current situation, here is the current UF repo structure : 

- UserFrosting
    - Core
    - Account
    - Admin
- Assets
- Cache
- Config
- Fortress
- I18n
- Session
- Support
- UniformResourceLocator

Compared to the proposed structure : 

- UserFrosting
- Core Sprinkle
- Account Sprinkle 
- Admin Sprinkle
- Framework
    - Assets
    - Cache
    - Config
    - Fortress
    - I18n
    - Session
    - Support
    - UniformResourceLocator

I also generated the current class structure of UF 4 : https://gist.github.com/lcharette/0d6db88d300690468436ed7555cf5ef2#file-1-currentstructure-md

Big questions that needs answers

1. Where do we draw the line between having classes in the core sprinkle vs. a vendor package?

First, it's important to note we do need a higher lever dependency. For a bare minimum, we need the SprinkleManager to be out of core, and ideally out of the skeleton repo too.

This question is valid for both UF core code and user code. Is there really a requirement for a user or UF extension to create a sprinkle (repo), load the code as a vendor package ?

I think the answer relies on one thing : Is the code UserFrosting specific, or can it be reused in another project ?

If the answer is yes, then I think a vendor package would make sense. But we have to keep in mind all other resources (locale, assets, routes, templates, etc.) still need to be accessible in a specific path (see point 4 about this).

In a functional standpoint, running phpunit on the skeleton repo should run all integration tests related to the content (controller, model, etc.). While all the "unit testing" of the individual component should be done independently.

I did a quick mockup of what the class/repo structure would be : 

- UserFrosting (Skeleton repo)
    - app/
        - cache/
        - logs/
        - sessions/
        - sprinkles/
            - …
        - storage/
        - .env
    - public/
        - index.php
    - vendor/
    - bakery

- UserFrosting Account Sprinkle
    - src/
    - tests/

- UserFrosting Admin Sprinkle
    - src/
    - tests/

- UserFrosting Core Sprinkle
    - src/
    - tests/

- UserFrosting-Framework
    - src/
        - Alert
        - Assets
        - Bakery
        - Cache
        - Config
        - Controller
        - Cerf
        - Database
            - Migrator
            - Model
            - Relations
            - Seeder
        - Error
        - Fortress
        - Http
        - I18n
        - Log
        - Mail
        - Router
        - ServicesProvider
        - Session
        - Sprunje
        - Support
        - Sprinkle
        - Tests
        - Throttle
        - Twig
        - UniformResourceLocator
        - Util
    - tests/

The full mockup can be found here : https://gist.github.com/lcharette/0d6db88d300690468436ed7555cf5ef2#file-3-proposedstructure-md

Some notes about this structure :

  1. The difference between the core sprinkle and the framework repo can be seen as :

    • All data structure stays in the sprinkle, while all the helper methods moves to the framework repo.
    • This means all Migrations, Model, Seeds, template, locale, assets, etc. stays in the sprinkle.

  2. Some of the UserFrosting/Framework content could be split into their own read-only repo, just like Laravel Illuminate packages… But that wouldn’t be required, as the skeleton repo would probably load the whole framework repo.

  3. The “framework” namespace wouldn’t be used for simplicity. So it would still be UserFrosting/Assets/…, UserFrosting/i18n/…, not UserFrosting/Framework/i18n. Only sprinkles would be differentiated with the UserFrosting/Sprinkles/ Namespace.

  4. At this point, I’m not sure how the bakery commands should be handled. Of course, bakery commands should be registered instead of discovered, which means we could put them wherever we want. They could be in the Framework (would make it easier to test) or a sprinkle. Assuming the Bakery class is in the Framework, then I guess it makes sense for command to be found there too. A good example would be the locale commands. They don't technically requires UF to run. One could clone the i18n repo and want execute those commands. Note Laravel works that way.

  5. Same goes for services provider. One thing important about service, currently my IDE doesn’t recognize $this->ci->translator as an instance of Translator since it’s actually loaded as a callback inside the provider class. We should implement a custom way to implement something that could take this into account, plus one which is testable.

In the end, I think the key is everything in the Framework repo needs to be self contained. They can't have dependencies on the content of the sprinkle (including Models), but it can have dependency on other framework component.

In other word, if a component form UF 4 can be moved to a distinct repo right now, it should be moved to the framework repo.

On the other hand, we also need to take into account dependencies on the locale strings, which would stay inside the sprinkle...

I see the Framework as the bare minimum UF needs to run, but also the one that dictate how it should run. That's why I think this should also apply for Interfaces, which should ideally be in the Framework.

2. How do we structure the Sprinkle namespace and directory structure if we add sprinkle vendor name ?

The directory structure would ideally be :

    - app/
        - sprinkles/
            - userfrosting/
                - core
                - admin
                - account
            - lcharette/
                - FormGenerator
                - MySite
        - storage
        - sessions
        - …
    - public/
    - vendor/

Current namespace for sprinkle is :

I see 3 options for the new sprinkle namespaces :

Option 1 :

Option 2 :

Option 3:

I suggest we stick to the first option, as it’s more “psr-esque”

3. Should vendor dir be in app/vendor/ or vendor/ ?

I think we should move vendor/ to the root repo to be consistent with 99% of the PHP world, especially when running command (ie. vendor/bin/phpunit vs app/vendor/bin/phpunit)

4. Should dependent sprinkles be loaded in vendor, and custom user code be left in app/sprinkles/ ?

For this one, I think we need some sort of control. I've talked about classes so far, but we still need to think about the other resources not in src/ : assets, locale, routes, config, etc.

See also

UF 5.0 Related issue / ideas / wishlist :

lcharette commented 4 years ago

One more thing... I'm not sure how to handle the current Account sprinkle stuff just yet. I don't see it in the main framework repo containing the Authenticator. We should be able to left it out of the whole stack if someone doesn't need it, or want to use it's own authenticator.

This brings to another idea that could be implemented. Instead of dropping everything in the framework repo, some feature could also be handled by a new sprinkle. However, this could bring another level of difficulties, in the UI side of things for instance.

For example, a Notification feature could either be baked into the Framework, but could also lives inside a sprinkle. My Breadcrumb sprinkle is a good example of this.

Maybe inside the list of classes / features I listed as potentially be inside the framework, some could be moved to a custom sprinkle, just like Account would exist...

lcharette commented 4 years ago

Ok I think I have an answer to my own question :

1. Where do we draw the line between having classes in the core sprinkle vs. a vendor package?

With time.

While with time the "framework" repo will probably get bigger and bigger as we move stuff from core to it, for the initial release it should contain the existing support repos plus System (main UF class + sprinklemanager) and Test : 

- UserFrosting-Framework
    - src/
        - Assets
        - Cache
        - Config
        - Fortress
        - I18n
        - Session
        - Support
        - System
        - Tests
        - UniformResourceLocator
    - tests/

This will allow the skeleton repo to exist, while allowing it to evolve with time.

As for the existing support repos, we already know having the many repos makes it difficult to develop, tests and release as they are dependent on each other and some needs to be synced to work (eg. set support to dev-develop to test it in other repos, and change it back later). So moving them to the framework repo solves this. Plus "SprinkleManager" depends on the locator

I think for 5.0, we can probably leave out sprinkle vendor "for now" and add it in 6.0 for instance.

All while replacing sprinkles.json with composer (and composer,dist), it will keep the same general class structure for an easier V4 to V5 migration.

So to resume :

lcharette commented 3 years ago

Ok guys, @alexweissman @Silic0nS0ldier : Can we at least get the support repo merged in a single Framework one now, and release this for 4.5 ?

Problem is the support packages are currently cross dependent :

Repo Depend on
Assets Support
Cache  
Config Support
Fortress i18n
I18n Support
Session  
Support URI
URI  
  1. I18n -> Support -> URI
  2. Assets -> Support -> URI
  3. Config -> Support -> URI
  4. Fortress -> I18n -> Support -> URI
  5. Support -> URI

I can't fix an issue in i18n without making a fix in UniformResourceLocator, but to make sure it's actually resolve the issue, I need to manage Support in-between with a temporary commit so it can be picked up by Github Action properly.

New "Framework" repo would be structure like this : 

Framework
  - src/
     - Assets
     - Cache
     - Config
     - Fortress
     - I18n
     - Session
     - Support
     - UniformResourceLocator
  - tests/
     - Assets
     - Cache
     - Config
     - Fortress
     - I18n
     - Session
     - Support
     - UniformResourceLocator
  - composer.json
  - etc.

Only issue, we'll lose the git history in the new repo for each independent repo, unless you know a way to conserve it somehow ?

Silic0nS0ldier commented 3 years ago

The current dependency graph is a little crazy. 100% it needs attention in UF 5 (or UF 6 if the scope is overloaded), with my thoughts being that UserFrosting\Support should be cast into eternal hellfire and package splits either revisited or abolished in favour of a monolithic library.

However this goes, switching to a project based structure (where 90% or more UF code is brought in by composer) should help quite a bit.

Oh, and this comment only relates to https://github.com/userfrosting/UserFrosting/issues/1095#issuecomment-753737063, I haven't read the rest of this thread.

lcharette commented 3 years ago

(Looks like preserving the git history could be done : https://medium.com/@checko/merging-two-git-repositories-into-one-preserving-the-git-history-4e20d3fafa4e)