laravel / ideas

Issues board used for Laravel internals discussions.
939 stars 28 forks source link

mix() Blade directive in app layout to support mix.version() by default #2080

Open michapietsch opened 4 years ago

michapietsch commented 4 years ago

Currently the asset() directive is used in the app layout stub to pull in js/app.js. So it provides a url, but does not pick up the Laravel Mix manifest to support versioning.

Because Laravel Mix may be involved in many cases, it would seem like a sensible default to use mix() instead. Going back to change the directive in app layout is an additional manual step. But I understand the asset() directive is the safe way to go, since the ui:auth command can be run on its own without a Mix build.

For me this was a stumbling block the first time I encountered it. So I was thinking about contributing. A separate layout stub seems too much. Maybe it's worth a note in the docs. Right now a beginner needs to go back to the Laravel Mix docs and closely look at how files need to be pulled in. There may be the false expectation that there can't be an issue with the scaffolding.

driesvints commented 4 years ago

Heya, since the mix helper is part of the framework I'm moving this issue to the ideas repo.

michapietsch commented 4 years ago

@driesvints Sorry, if that's a bit unclear: I was talking about the app layout template stub that comes with ui, but maybe I misunderstand.

ryangjchandler commented 4 years ago

I think the use of the asset() helper is perfectly valid, since the mix() helper will throw an exception if the mix-manifest.json file can't be found. It's one less thing the developer needs to do before viewing their application

michapietsch commented 4 years ago

@ryangjchandler I get your point. That's why I didn't propose to use mix() right away.

There's a percentage of users who use the scaffolding with e.g. Vue, and they will run into unexpected behavior without an exception.

I only refer to this use case, and of course the existing stub has to stay as it is for other cases.