statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
3.99k stars 526 forks source link

Scripts route need to be configurable #1742

Closed jonassiewertsen closed 4 years ago

jonassiewertsen commented 4 years ago

In addon context, it would be nice to configure the scripts path, to keep clean vendor folders.

Screenshot 2020-04-26 at 22 38 45

Right now, it's not possible to configure the path for scripts. Statamic will automatically create two folders: vendorname/packagename/scriptname.js Like in my example, and as done by Statamic, it would be nice to could follow: customname/scriptname.js

Environment

Statamic version: 3.0.0.-beta-27

PHP version: 7.4

Install method (choose one):

jasonvarga commented 4 years ago

It’s doing both? It shouldn’t be.

jonassiewertsen commented 4 years ago

No, Statamic is only publishing one file.

What Statamic does give me: jonassiewertsen/statamic-butik/js/statamic-butik.js

My Service provider does look like:

protected $scripts = [
        __DIR__ . '/../public/js/statamic-butik.js',
    ];

I would like to customize the public path, to publish it in another directory like butik/js/statamic-butik.js

Then it would match my other files and the behavior of other packages.

jasonvarga commented 4 years ago

That's the way it is. JS goes in public/vendor/yourvendorname/yourpackage/js/filename.js

jonassiewertsen commented 4 years ago

Screenshot 2020-04-28 at 21 15 31

Statamic itself does not follow this pattern. A similar pattern for the vendor files of Statamic addons (including Statamic) would be great IMHO.

jasonvarga commented 4 years ago

I'm not following. statamic is the vendor name.

jonassiewertsen commented 4 years ago

Following this pattern, it would be public/vendor/statamic/statamic/js

jasonvarga commented 4 years ago

cp is what we're considering the package there. It's also not an addon anyway.

I'm sorry, I don't see what the problem is. 🤔

jonassiewertsen commented 4 years ago

Like in my example, and as done by Statamic, it would be nice to could follow: customname/scriptname.js

Having public/vendor/yourvendorname/yourpackage/' as default but having the option, to customize it to likepublic/vendor/CUSTOM-PATH/`

Defining the js scripts via the

protected $scripts = [
        __DIR__ . '/../public/js/statamic-butik.js',
    ];

It is a nice little helper. I am missing the opportunity to customize the resource path though, as it comes with the standard Laravel way of publishing scripts.

$this->publishes([
     __DIR__.'/../public/js' => public_path('vendor/butik/js'),
], 'butik-resources');

The definition of the public_path as optional argument would be great. Using the helper is great to publish files automatically to the cp, like for custom field types. Without the custom public_path, it is messing with my existing structure as seen above.

If you think it would make sense, I am happy to create a PR.

jasonvarga commented 4 years ago

You're free to add $this->publishes() in your own boot method for extra files. The $scripts are what will automatically be loaded in the control panel. That's the convention we're going with.

Your "existing structure" - why does that exist? Why can't you just use the convention?

jonassiewertsen commented 4 years ago

The convention is vendor/packagename and not as automated via Statamic vendor/vendorname/packagename

It would be nice to keep the Laravel convention: vendor/butik/ AND not vendor/jonassiewertsen/butik as automated by Statamic.

I just checked a few other repositories and couldn't find other conventions.

Laravel Telescope is staying with vendor/packagename: Laravel Telescope service provider

Marcel Pociot as well: Web tinker service provider

Spatie as well: Spatie Laravel enum service provider

jonassiewertsen commented 4 years ago

The Laravel docs are describing the same convention

jasonvarga commented 4 years ago

We can change the convention but I still don't see why it matters. It's a pretty transparent operation. The files get copied and you move on with your day.

The extra subdirectory with the vendor name is to protect two packages with the same name. Pretty rare for sure, but it could happen.

jonassiewertsen commented 4 years ago

The addon is already out there. To keep my files consistent, what I think is really important, I would need to relocate all

to keep up with the Statamic convention. Making the public path customizable would give a lot of flexibility, which is needed in some cases.

The perfect solution would be, to keep the Laravel convention public/vendor/packagename but to make the public_path customizable.

Such a breaking change would be much easier in the Beta phase than later.

jasonvarga commented 4 years ago

I understand that you were manually placing stuff in /vendor/butik, but Statamic was expecting /vendor/jonassiewertsen/statamic-butik. If you had just put those manually added files in that subdirectory, it would have been consistent in your package.

ie change this https://github.com/jonassiewertsen/statamic-butik/blob/master/resources/views/web/layouts/shop.blade.php#L9 to href="/vendor/jonassiewertsen/statamic-butik/css...

Yes, it would be a little different from other regular Laravel packages, but still all in one place.

Anyway, the convention is now just /vendor/{package}. I like to follow Laravel conventions where possible so this is 👍 , as much as this conversation seems like I've been arguing with you 😄

jonassiewertsen commented 4 years ago

This feels much cleaner to me and nice to follow the Laravel convention.

Little things do matter! I am appreciating the change. 🚀