Support dev server mode w/ HMR

[jonaskuske]

Currently kirbyup only allows building the plugin, either for production or using the --watch option, with full rebuilds on change.

Not sure if it's feasible, but a dev server mode with HMR would be great, especially for Vue components!

[johannschopplich]

Hi Jonas,

indeed, that would be great. It's extremely complicated tho, in my opinion.

It would be easy, if we would build the Kirby Panel ourselves. As a matter of fact, you can do so yourself and build your own custom Panel bundle with your components baked in. Then you'll benefit from all of Vite's glory.

But since the Kirby Panel has already been built, we execute a bundled and almost "locked" Vue instance. The only entrypoint is Kirby's plugin system. Kirby handles the JS injection. In order to provide HMR, we would have to create a dev server and build a static index.js for Kirby to read anyway. Is partly HMR for a component inside a Vue bundle even possible? My only solution at the moment would be to reload the page once chances are made to your components.

Honestly, I don't know how to pull that off. Not even considering how to handle the state inside those components and save them between hot updates.

Feel free to give it a shot! If you create a POC, I'd be happy to help integrating. For now, the use case and feature request exceeds my time and use case.

Thanks for the input.

[jonaskuske]

For now, the use case and feature request exceeds my time and use case.

Yeah, totally get that! :)

I explored this a little and you're right, without some changes to Kirby it doesn't really work. If a plugin is served by Vite it is served as native JS modules. Kirby needs the plugin to be executed before the panel's main index.js loads, however modules execute asynchronously so a separate <script> tag doesn't work. The plugin module(s) need to become part of the index.js's module graph so it can wait for them to load first, which is something Kirby needs to support in order to proceed.
I think this might be a worthwhile and relatively non-invasive change and implemented it here: getkirby/kirby@a23e357

With that in place, we can use a plugin entry file that "relays" the request to a Vite development server started by kirbyup:

// plugins/kirby-example-plugin/index.mjs

import 'http://localhost:5173/src/index.js'

Now we got working HMR! Almost. For CSS changes it works, which is quite nice on its own, and it also works for <template> changes. (those swap the render function and force a re-render, but keep the component instance)
But when the <script> part of a component is edited, the entire component is reloaded, which can cause errors. Kirby manipulates the options of plugin components "from the outside": https://github.com/getkirby/kirby/blob/main/panel/public/js/plugins.js
Vite doesn't know about these manipulations, so when the HMR runtime reloads the component and switches to a newer component instance, this instance only includes the options from the component definition, not the ones Kirby patched-in later. Due to this, a section component doesn't include the section mixin: calls to this.load() throw errors.

The easiest way to prevent such errors is to enable HMR only for CSS changes and reload the page if the JS/template was changed. This is how my current HMR implementation for kirbyup works: https://github.com/jonaskuske/kirbyup/tree/feat/hmr

It should also be possible to discern JS updates that cause the HMR runtime to refresh (good) from updates that cause a component reload (bad), and only trigger a page reload for the latter. However whether to refresh or to reload is only decided at runtime, so the only way I can currently think of is patching the HMR runtime code, something like:

// page reload instead of hot component reload
__VUE_HMR_RUNTIME__.reload = () => import.meta.hot.invalidate()

Then we'd have HMR for the <template> and <style> section.

If Kirby where to change its plugin loading behavior so it doesn't manipulate component definitions, we could have full HMR. Not sure about extends, but the mixin could be registered as a global mixin, for example. But that aside, HMR just for <style> or for <style> and <template> are already worthy DX improvements on their own. kirbyup could also use something like vite-plugin-live-reload to force a page refresh when a plugin's PHP file changes, to make the HMR plugin development experience complete. :)

[johannschopplich]

I'm blown away by your approach, Jonas. A serious Vite pro is in the house! Amazing contribution. 👏

Will look into it next week. Looks stunning so far. Great improvements! We'll put that into the core definitely.

[jonaskuske]

Heh, thank you! 😊

Just committed the change so template refreshes work with HMR and only instance reloads cause a full page reload: jonaskuske@fcf6fde

💡 Example repo to try it out: https://github.com/jonaskuske/kirbyup-hmr-playground

Documenting outstanding questions and todos here so they can be discussed later:

Kirby Core

• Loading plugin module scripts requires top level await, since normal imports can't be wrapped in try/catch and we don't want a single failing plugin to break the entire panel. It has higher browser version requirements:
  Current:         Chrome 63+, Firefox 67+, Safari 11.1+, Edge 79+, Opera 50+
Top level await: Chrome 89+, Firefox 89+, Safari 15+,   Edge 89+, Opera 75+
  Options:
  • Raise the browser version requirements for the panel
  • Make plugin modules opt-in (e.g. 'plugins.modules' => true)
  • Use UA sniffing and only run/serve module loading code for supported browsers
  • ...or maybe I'm missing something and this can be achieved with static imports, without top level await?
    
    With the first option, index.mjs entries could become a feature that's also supported for normal production plugins, with the other options it'd be a tweak that enables a better plugin authoring experience, but shipped plugins would continue to need a non-module index.js.
• Currently the mjs loading script is only injected into the production panel JS and doesn't work when the panel itself is being developed and loaded from a Vite dev server
• Add documentation
• Tests?

I've submitted getkirby/kirby#4541 in the Kirby repo to discuss Kirby-related changes there. :)

Kirbyup

• Make sure asset loading works (might need additional Vite config options, haven't tried it yet)
• Make kirbyup serve the default for the dev script (feat: add support and docs for kirbyup serve getkirby/pluginkit#12)
• Reload functionality for changes to .php files
• Add proper command descriptions in cac for kirbyup --help
• Add documentation
• Tests?

[jonaskuske]

The required changes to Kirby are merged and slated for release in 3.7.4 🚀🚀

I'll be on holiday soon and not sure if I'll manage to write the tests that will complete the PR before that, but either way this feature should be live fairly soon! :)

[johannschopplich]

Nice! I think the PR is pretty solid and extensively tested by you. Even tho we don't have automated tests yet, I intend to release a beta of the new kirbyup version (2.0) for user's to test and catch up with tests later. Would you be OK with that approach?

I'm gonna be on vacation soon as well. Perfect timing.

[johannschopplich]

Reopening since automatically closed by GitHub. Automated tests are still missing.

[jonaskuske]

Heh, and the fix for the Vue SFC compiler has landed as well: vuejs/vue#12732 (comment)! 🎉

And yep, definitely okay with that approach! I don't think it's a breaking change though, npx kirbyup <src> and npx kirbyup <src> --watch should behave exactly the same. So this would also be fit for a 1.4 feature release, unless I'm missing something?

---

As far as actual breaking changes go, I was thinking about re-evaluating the need for the thin abstraction over Vite vs. using Vite's own config resolving mechanism (and CLI commands?) directly. Config with custom option could look like:

// vite.config.js
export default defineConfig({
  resolve: { alias: { insteadOf: 'kirbyup.config.js#alias' }},
  plugins: [kirbyup({ customOption: true })]
})

Could simplify the code base and lead to less code to maintain.

As for CLI commands, we could either

• Only use named commands. (kirbyup serve|dev, kirbyup build). Would allow for a little cleaner CLI help output in cac as you can add examples and descriptions for both commands to kirbyup --help and then have separate command-specific help for kirbyup <command> --help. (can add a deprecation warning to kirbyup <src> in a minor to make the change easier)
• Align the commands with Vite's own commands:

{
  "dev": "kirbyup <src>", // alias dev, serve?
  "build": "kirbyup build <src>"
}

• Use Vite commands directly: vite dev, vite build. The kirbyup binary could forward all CLI args to vite and make sure the kirbyup plugin is added, so npx kirbyup usage continues to work.

But those are just some ideas that came to my mind while working on this PR, also inspired by the changes Svelte Kit went through – might be unnecessary, and even if worth considering, definitely material for a separate issue/PR.