winter icon indicating copy to clipboard operation
winter copied to clipboard

[WIP] Integrate Vite & Rework Mix

Open jaxwilko opened this issue 1 year ago • 14 comments

This PR implements support for Vite side by side with Mix. All exsiting Mix functionality has been retained, but all the commands have been abstracted to allow for re-use by Vite. This allows for full support of both compilers, at the same time.

Modified

  • Commands
    • mix:compile, mix:watch, mix:install, mix:list all now rely on abstract Asset* commands and inherit 90% of the functionality to allow for Vite* commands to reuse the same code.
    • mix:run & mix:update - these were used for interacting with npm via the Winter CLI, these have been renamed to refelect their function to npm:run & npm:update. Alias have been set to refer back to their previous names incase anybody is using them in scripts.

New

  • Commands
    • vite:compile - shares args with mix:compile called vite build under the hood
    • vite:watch - shares args with mix:watch called vite under the hood (defaults to "watching")
    • vite:install - shares args with mix:install allows for configuring package.json workspaces and installing required global packages
    • vite:list - shares args with mix:list
    • vite:config - new allows for setting up stub files
    • mix:config - new allows for setting up stub files
  • System\Classes\PackageJson - Now provides a consistant interface for modifying package.json files rather than using json_encode/json_decode everywhere
  • Twig function vite() has been added, this works similarly to Laravel's @vite() in blade, however it required the base path to also be passed.
  • vite:watch, vite:compile, mix:watch & mix:compile now support --disable-tty which passes output back to the Symfony Process allowing for capture by callers (useful in testing).

Docs

Add a vite.config.js file to whatever package (plugin, theme, module) you want, then expect the same behaviour as you would with the mix:* commands, but with vite:* instead. This can be done for you via the vite:config command.

When using Vite, to load your assets in your markup you need to use the new vite() function. It takes 2 arguments, an array of entry points, and the package name for where the assets are loaded from.

This should end up looking like:

{{ vite(['assets/css/theme.css', 'assets/javascript/theme.js'], 'theme-example') }}

Note: The default vite port is 5173, you should ensure that port is open. If you are running multiple vite:watch commands at the same time, the port will increment with each watch, so open those ports too if required.

Vite can also be used in the backend:

<?= \System\Classes\Vite::tags(['assets/css/example-plugin.css'], 'example.plugin') ?>

TODO

  • [ ] Testing
    • [ ] Fresh install
    • [ ] Fresh plugin
    • [ ] Fresh theme
    • [x] Linux
    • [ ] Mac
    • [ ] Windows
  • [ ] Vite TestCases
  • [x] *:config TestCases

jaxwilko avatar Jun 05 '24 12:06 jaxwilko

Couple of initial comments:

  • Perhaps we could add *:build and *:dev command aliases to *:compile and *:watch commands, respectively. Both terms are more commonly used for JS libraries.
  • If we're going to be separating the run and update commands under an npm namespace, then install should probably go under there too. Perhaps we should abstract that command to be able to define which path the user wants to go down (ie. install Mix or install Vite).
  • With the vite command in Twig, I would think we can determine the theme's base path automatically. Is there any specific reason why we need to define it explicitly?

bennothommo avatar Jun 05 '24 22:06 bennothommo

@bennothommo

  • Perhaps we could add *:build and *:dev command aliases to *:compile and *:watch commands, respectively. Both terms are more commonly used for JS libraries.

100% agree, makes sense.

  • If we're going to be separating the run and update commands under an npm namespace, then install should probably go under there too. Perhaps we should abstract that command to be able to define which path the user wants to go down (ie. install Mix or install Vite).

Install isn't just installing packages, it's validating the configs are set and adding packages to workspaces, then adding deps if required, I kinda like how it is atm, although maybe it should be named something different.

  • With the vite command in Twig, I would think we can determine the theme's base path automatically. Is there any specific reason why we need to define it explicitly?

Yes for theme, maybe for plugins, and maybe for somebody adding assets from a plugin to a theme (idk why but they could). All these edge cases are solved by just setting the same base that we pass when executing the command. Basically the php side needs to match up with the js web socket server. We could default to a guess, but it may cause more confusion when it fails than just setting a path and being done with it.

jaxwilko avatar Jun 05 '24 23:06 jaxwilko

Thanks for the feedback @jaxwilko.

Install isn't just installing packages, it's validating the configs are set and adding packages to workspaces, then adding deps if required, I kinda like how it is atm, although maybe it should be named something different.

That's cool by me. I just would like a rational and natural way for someone to "update" their dependencies - especially with the frequency of JS libraries and their dependencies being affected by security vulnerabilities. I suppose that npm:update would work - perhaps allowing us to reserve vite:upgrade for doing more thorough upgrades to Vite if, say, down the track we need to make configuration changes as part of an upgrade process (ie. allowing the command to do major upgrades to Vite).

Yes for theme, maybe for plugins, and maybe for somebody adding assets from a plugin to a theme (idk why but they could). All these edge cases are solved by just setting the same base that we pass when executing the command. Basically the php side needs to match up with the js web socket server. We could default to a guess, but it may cause more confusion when it fails than just setting a path and being done with it.

I've been thinking about this exact scenario for some time - pretty much since we introduced Mix. I think the easiest way we can move forward on this is that we have to consider that each plugin and theme is responsible for its own build and assets and that's it. The fact is, not everyone is going to write plugins and components that work nicely with every type of build used by themes, especially when our goal with the CMS side is that "people can use whatever front-end framework they want".

As you said too, people can have the plugins (or at the very least, the components) inject assets into the theme when used. I still would argue that it would be most likely that the "entrypoint" to these assets would still reside in the theme if they built the plugins themselves. For third-party plugins, it's fair game for them to have their own build process and you simply use the compiled assets (or perhaps the build process in the theme could concatenate/minify these assets into the theme build). Either way, I think they're both out of scope for the vite filter you would be introducing.

So I wonder if we can just simplify the vite filter to assume all paths are relative to the theme, and make it so that the entrypoints always exist within the theme (these entrypoints can however import plugin assets however they wish).

Thoughts on this?

bennothommo avatar Jun 06 '24 14:06 bennothommo

I don't think vite will play ball, unless we only enable vite for themes and not plugins (which was my first thought before I was able to get plugins working too).

By all means have a play with it, but I think currently just adding the base covers all possible situations with the most flexibility.

jaxwilko avatar Jun 06 '24 14:06 jaxwilko

@LukeTowers @bennothommo the param ordering for getPackages() has been swapped. I've also added some extra helpers to CompilableAssets & PackageJson to simplify logic in some commands.

jaxwilko avatar Jun 07 '24 09:06 jaxwilko

@mjauvin do you feel like having a play around with this to help test it?

LukeTowers avatar Jun 08 '24 04:06 LukeTowers

You all know the love I have for npm... why do you even ask? 😂

mjauvin avatar Jun 08 '24 06:06 mjauvin

@bennothommo had a thought, what if we swap the "base path thing" for the package name, i.e. theme-example or winter.test, then resolve the base via CompilableAssets.

That way it makes a lot more sense as you're "loading entry files from this package".

Any thoughts?

jaxwilko avatar Jun 09 '24 03:06 jaxwilko

That sounds a lot better. I still think it can default to the theme base path, but yeah having it allow the theme and plugin codes that Mix uses would give that extra flexibility in a simple way. 👍

bennothommo avatar Jun 09 '24 03:06 bennothommo

You all know the love I have for npm... why do you even ask? 😂

That's why I ask 😂

LukeTowers avatar Jun 11 '24 05:06 LukeTowers

I've put together a simple testing guide to try out vite + the new config command.

Set up a testing plugin, configure it and add stubs:

./artisan create:plugin Example.Test
./artisan create:component Example.Test Testing
./artisan vite:config Example.Test --tailwind --stubs
./artisan vite:install

echo  -e "<p class=\"text-red-900\">hello world</p>\n{{ vite(['assets/css/example-test.css'], 'example.test') }}" > plugins/example/test/components/testing/default.htm

Enable the component in the plugin:

return [
    \Example\Test\Components\Testing::class => 'testing',
];

Add the component to one of your pages:

...
[testing]
...
==
...
{% component testing %}
...

Run vite:

./artisan vite:watch example.test

jaxwilko avatar Jun 11 '24 10:06 jaxwilko

I've added a helper so you can run vite in the backend too.

<?= \System\Classes\Vite::tags(['assets/css/example-plugin.css'], 'example.plugin') ?>

jaxwilko avatar Jun 11 '24 14:06 jaxwilko

@jaxwilko does it make sense to add support to the AssetMaker trait as well?

LukeTowers avatar Jul 03 '24 02:07 LukeTowers

Relies on: https://github.com/wintercms/storm/pull/183

jaxwilko avatar Jul 03 '24 15:07 jaxwilko

I've tested this with an existing theme in a completed project, it functioned as expected, however as that project uses https by default it caused some mixed content issues. This could be resolved by switching back to http only or via a fix from here: https://stackoverflow.com/questions/69417788/vite-https-on-localhost or sticking vite behind nginx. Either way it falls outside the scope of this PR but will be necessary to document.

jaxwilko avatar Jul 10 '24 14:07 jaxwilko

@jaxwilko I've given this a test - it's really cool, and worked nicely as far as I could tell.

However, if I am to be brutally honest, I still prefer the configuration API of Mix. It's an API that someone who detests JavaScript (ie. @LukeTowers :stuck_out_tongue:) would still feel comfortable using. Vite is obviously leaps and bounds more easy to wrangle than Webpack, but it's still very declarative for all but the most simple of cases (someone using the base config and simply using the vite() method in their templates).

As soon as someone needs to do something complex, you're back into defining huge config and plugin objects in the config file. There's no way (I think) that I can do my Monaco Editor build with Vite currently.

I'm not saying we should do this now given that your intention is to still allow Mix to work, but since the underlying laravel-mix library is all but abandoned at this point, I'm thinking that it would be nice down the track to create a wrapper to configure Vite in the same way Mix did for Webpack, if that's at all feasible.

bennothommo avatar Jul 11 '24 00:07 bennothommo

@bennothommo

I still prefer the configuration API of Mix. It's an API that someone who detests JavaScript (ie. @LukeTowers 😛)

The beauty of Winter is we can support both (and with all the abstraction I added, we can now easily support FrameworkX+n when it becomes the default next month). If I'm honest I used to write Webpack configs before writing that mix plugin years back so either mix or vite configs don't bother me much :joy:.

I'm thinking that it would be nice down the track to create a wrapper to configure Vite in the same way Mix did for Webpack

Rollup vs. Webpack are very different, the way they opperate is different and it comes with some advantages and some disadvantages.

One issue I found earlier is that if you're running a site in https, then vite will cause issues as it's webserver is http by default, you need to go out of your way to either wrap it under nginx or use one of the many plugins that exist to fix the issue for you (or manually configure vite with your own certs).

I'm sure there are many more similar issues due to the differing paradigms between Mix & Vite, which is why I want to offer support for allowing plugins, themes and registerable things (i.e. direct calls to CompilableAssets) to manage a vite/mix "package", but kind of be hands off at that point, allow for whatever configuration the user wants to do with vite/mix but not get in their way.

The way I see it, if they have an issue with vite, they will google "Vite not work with this thing", which will probably get a result that tells them what to put in their config to fix it. If we change that to "winter-vite-wrapper thing not work" then they will likely get far less results and the issue will fall on us to support our custom extension.

but since the underlying laravel-mix library is all but abandoned at this point

Till it starts causing issues I'd like to keep supporting it, I think Vite will work very well for frontend themes but I'm not sure how well it'll work in the backend and I think Mix may generally work better due to it's simplicity (that said, @LukeTowers made be add a AssetMaker::addVite() method which does work nicely). We're still supporting Assetic after all.

As soon as someone needs to do something complex, you're back into defining huge config and plugin objects in the config file. There's no way (I think) that I can do my Monaco Editor build with Vite currently.

Ping me when you're next free on discord and we can try and get it working :). I've tried out 2 production sites converting existing mix configs over but they weren't doing much clever stuff so it would be a good test to find any edge cases.

jaxwilko avatar Jul 11 '24 04:07 jaxwilko

@jaxwilko is there a docs PR?

LukeTowers avatar Jul 11 '24 14:07 LukeTowers

@LukeTowers working on it atm :)

jaxwilko avatar Jul 11 '24 15:07 jaxwilko

Docs PR: https://github.com/wintercms/docs/pull/200

jaxwilko avatar Jul 12 '24 15:07 jaxwilko

Just waiting for @bennothommo to weigh in on the remaining two items (package.json being excluded from the source export & moving the PackageJson class into either the Storm Parse package or the Laravel Config Writer package).

LukeTowers avatar Jul 15 '24 06:07 LukeTowers

@jaxwilko @bennothommo are there any remaining items before we can merge this?

LukeTowers avatar Jul 16 '24 05:07 LukeTowers

Not that I'm aware of.

bennothommo avatar Jul 16 '24 05:07 bennothommo