laravel-streams / streams-core

Streams is an open-source web application engine for Laravel.
https://streams.dev
Other
169 stars 99 forks source link

RFC: Proposal - Root project asset compilation #725

Closed RobinRadic closed 3 years ago

RobinRadic commented 3 years ago

Request For Comments

CPFE: control panel front-end RPAC: root project asset compiling PPAC: per-package asset compiling

Summary

Switch to root project asset compiling (RPAC) instead of per-package asset compiling (PPAC). This means:

Basic example

git clone https://github.com/laravel-streams/streams.dev
cd streams.dev
git checkout feature/webpack-mix-root-based-bundling
composer install
yarn
yarn dev

Produces something like:

┌────────────────────────────────────────────────────────────────────────────────────────┬───────────┐
│                                                                                   File │ Size      │
├────────────────────────────────────────────────────────────────────────────────────────┼───────────┤
│                                                              vendor/streams-vendors.js │ 863 KiB   │
│                                                         vendor/streams/core/js/core.js │ 243 KiB   │
│       …treams/ui/css/chunk.packages_streams_ui_resources_scss_inputs_markdown_scss.css │ 12.4 KiB  │
│                                                           vendor/streams/ui/css/ui.css │ 5.09 KiB  │
│                   vendor/streams/ui/js/chunk.node_modules_easymde_src_js_easymde_js.js │ 688 KiB   │
│                         vendor/streams/ui/js/chunk.packages_streams_ui_lib_atest_ts.js │ 662 bytes │
│  vendor/streams/ui/js/chunk.packages_streams_ui_resources_scss_inputs_markdown_scss.js │ 621 bytes │
│                                                             vendor/streams/ui/js/ui.js │ 303 KiB   │
└────────────────────────────────────────────────────────────────────────────────────────┴───────────┘
yarn prod

Produces something like:

┌────────────────────────────────────────────────────────────────────────────────────────┬───────────┐
│                                                                                   File │ Size      │
├────────────────────────────────────────────────────────────────────────────────────────┼───────────┤
│                                                                             mix.js.map │ 1.24 KiB  │
│                                                              vendor/streams-vendors.js │ 167 KiB   │
│                                                         vendor/streams/core/js/core.js │ 74.9 KiB  │
│                                                    vendor/streams/ui/css/chunk.746.css │ 12.4 KiB  │
│                                                           vendor/streams/ui/css/ui.css │ 4.17 KiB  │
│                                                      vendor/streams/ui/js/chunk.442.js │ 300 KiB   │
│                                                      vendor/streams/ui/js/chunk.746.js │ 120 bytes │
│                                                      vendor/streams/ui/js/chunk.785.js │ 213 bytes │
│                                                             vendor/streams/ui/js/ui.js │ 84.3 KiB  │
└────────────────────────────────────────────────────────────────────────────────────────┴───────────┘

Production build:

┌────────────────────────────────────────────────────────────────────────────────────────┬───────────┐
│                                                                                   File │ Size      │
├────────────────────────────────────────────────────────────────────────────────────────┼───────────┤
│                                                                             /js/app.js │ 4.39 KiB  │
│                                                                          css/theme.css │ 1.87 MiB  │
│                                                              vendor/streams-vendors.js │ 335 KiB   │
│                                                           vendor/streams/api/js/api.js │ 4.6 KiB   │
│                                                         vendor/streams/core/js/core.js │ 25.6 KiB  │
│       …treams/ui/css/chunk.packages_streams_ui_resources_scss_inputs_markdown_scss.css │ 12.4 KiB  │
│                                                           vendor/streams/ui/css/ui.css │ 4.17 KiB  │
│                   vendor/streams/ui/js/chunk.node_modules_easymde_src_js_easymde_js.js │ 333 KiB   │
│  vendor/streams/ui/js/chunk.packages_streams_ui_resources_scss_inputs_markdown_scss.js │ 229 bytes │
│                                                             vendor/streams/ui/js/ui.js │ 26.1 KiB  │
└────────────────────────────────────────────────────────────────────────────────────────┴───────────┘

Drawbacks of the PPAC approach

Advantages of the PPAC approach

Drawbacks of the RPAC approach

Advantages of the RPAC approach

RobinRadic commented 3 years ago

Any comments or questions are welcome

RobinRadic commented 3 years ago

CP

Most of this assumes a full merge of the feature/webpack... branch

laravel-streams-mix-extension

The current implementation uses yarn workspaces to make laravel-streams-mix-extension located in packages/laravel-streams-mix-extension.
If it wasn't located there, it will be pulled from the NPM repository. You mentioned something along the lines of including that in the core?

asset handling

assets.blade.php

  1. Change

    <script src="/vendor/streams/core/js/index.js"></script>
    <script src="/vendor/streams/ui/js/index.js"></script>

    Into
    {!! Assets::collection('scripts')->tags() !!}

  2. ServiceProvider asset handling

    protected function extendAssets()
    {
    $this->publishes([
        __DIR__ . '/../resources/public' => public_path('vendor/streams/ui'),
    ], 'public');
    Assets::addPath('ui', 'vendor/streams/ui');
    Assets::add('head.scripts', 'ui::js/head_script.js'); // will be published
    Assets::add('scripts', 'ui::js/ui.js');
    Assets::add('styles', 'ui::css/ui.css');
    }
  3. Dependency sorting The issue that arises is that Laravel adds service providers in a alphabetical manner. This means that if you add a script in streams/api, it will be added before the streams/core scripts. This leads to errors if you have import {anything} from 'streams/core in streams/api. A solution might be

    Assets::add('scripts', 'api::js/api.js', ['core::js/core.js']); 

    You could check the Sorter i created.

PHP 7.3 support

owenvoke/blade-fontawesome only supports "php": ">=7.4". This could be 'solved' with a composer option --ignore-platform-reqs.
and it might work adding it to composer.json it's config settings like "ignore-platform-reqs": true

RobinRadic commented 3 years ago

RyanThePyro at 2:31 PM: Question - where does the package MIX come into play for stashing things in resources/public for publishable assets with root package management?

Radic at 2:31 PM: mix package? doesnt exist, you mean laravel-streams-mix-extensions or laravel-mix?

RyanThePyro at 2:32 PM: It is more of a "It's already done, just in PHP" lol - and the translation of that to TS is slow for me - might be cool if you are open to it to PR it and I can hack on it from there. It's more about time then implementation - the implementation model is before us in PHP Ya with the root stuff - laravel-streams-mix-extensions - that publishes to public for the website - which is great for core package maintainers n stuff - but what about publishing those assets in the respective packages (streams/core/resources/public, streams/ui/resources/public for shipment?

Radic at 2:33 PM: I'm mostly not touching the PHP stuff. I don't really know what your direction is heh. But these kinda things are pretty much written in stone already ah no, that needs to happen by developers itself. not core developers but the ones implementing it you only need npm

RyanThePyro at 2:35 PM: PHP stuff is done - I was just saying that structure can be mimic'd

Radic at 2:36 PM: i could possibly make it so that it exports to vendor/streams etc. but that's not gonna work properly

RyanThePyro at 2:41 PM: Ya, this has always seemed less of a core thing and more of a front-end integration thing. Without that functionality - this is not helpful to me directly, I think. Because I am never building/compiling for my own project hardly. I am building for others - the output must be distributable. Problem is the duplication - which might be where your stuff comes into play And I want the shit like hot reloading But, I still have to have a package mix file. And maybe that was gonna be the case anyways. But if I have to go in and run production there anyways - seems to defeat the point right? To some degree at least. Am I correct in that logic?

Radic at 2:56 PM: Let me first disagree op your first point :yum:

1) It's not a front-end thing. In programming in general you don't put compiled stuff on git. Besides, you are building/compiling your own project every time. If you wouldn't want that you would distribute it with vendor map, how does that sound in your ears? Pure blasphemy right?

2) Laravel, per default doesn't ship compiled resources either. You have to go through webpack mix. It's not distributable.

3) It's the standard in web-development.

Exporting to vendor/streams/core/resources/public is not possible due to not knowing what pacakges are actually installed. So it's prone to errors There are however still resources that needs to be published with artisan vendor:publilsh --tag=public Which i could easily fit into webpack But in the end though, the option still remains for third-party developers to completely ignore this webpack method It's still possible to use pre-compiled assets

RyanThePyro at 3:04 PM: What about something like Spatie Web Tinker or Laravel Telescope

Those are compiled and distributed.

I am afraid this might be a paradigm issue on my end.

Does this inherently complicate the root project? Because now the dependencies of ALL the streams packages have to align with your base package requirements right? 3) It's the standard in web-development.

In this particular context? I think what we are doing is not standard, but standards can be applied and then coupled together via cohesive design via the API n shit.

But, it seems this flies just slightly in the face of a distributed and decoupled control panel utility/service. Im gonna paste part of this convo and the PR to masterminds

Radic at 3:08 PM: Just paste it all, lets not leave out any context. But lets finish it first :kissing_heart:

RyanThePyro at 3:09 PM: Haha ok!

Trying to whittle down my core concerns..

I think it's the duty of the innocent user of Laravel Streams features.

Also think about bolting onto existing projects - this makes it a little more involved in some ways if you HAVE to compile the CP The CP should just be - in my ideal world. You configure it and can tap into it AND extend it if you like. But it's outside of the core concerns of your build.

What about something that pushes production built public assets back into their vendor directory? So you would re-bundle it after you build from root by pushing public/vendor/streams/core/* back into vendor/streams/core/resources/public I gotta jet with the kids but am gonna paste this and open up discussion - please put your further points in there! This is good.

I want my cake and I wanna eat it too.

Radic at 3:14 PM: Yeah sure man, il go ahead and further argument it

RyanThompson commented 3 years ago

I've set up https://www.npmjs.com/org/laravel-streams and invited you - please submit that package when you can!

RyanThompson commented 3 years ago

Dumping thoughts here for posterity.

After careful consideration and pulling this code in and toying with it, I have concluded that RPAC is in direct contradiction to one or more of the project's fundamental values and principles.

I will work on some material to help reconcile this further against those values and principles but here are some top-level issues I have verified when working through this.

Minimal impact policy. The project's goal is to have as little as ZERO impact on the root project because it's not our project. Streams is a utility first, now. It should not dictate root-level anything for any project. Requiring root-level compilation to get anything stood up really is a non-starter issue. Requiring it to customize anything is also undesirable.

Keep complexity low. This approach really quickly balloons out of control with nested dependencies and compiling issues related to trying to make everything play nicely. If you (a developer) want to utilize this approach for your project, more power to you. I hope to make our assets consumable in a way that maximizes developer efforts in this way, with our default position (decoupled, self-contained, truly plug-and-play).

Adhere to fundamental practices and design. Thus far, we have been able to provide massive performance and functional versatility with very simple mechanics and software design. I would argue that RPAC is neither. It adds complexity, adds dependencies, adds required input and action on the developer and causes impact to their workflow. This combined with my belief that it doesn't actually solve a problem is why the project will stick to PPAC and distributing publishable and easily consumable assets.

I will continue working on the values/principles area of the website because these things need to be reconciled against for all RFCs, features, etc. Having them out and in the open for everyone to consider will be good.