feat(vite): Support default export of render in Vite HMR wrapper

[kwentine]

Fixes #586

It is a rough starting point inspired from ./anywidget/src/widget.js helpers, with warnings abridged.

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 0fbb393290d6809e727f2220cb6f74c2df322be9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

| Name | Type | | --------------- | ----- | | @anywidget/vite | Minor | | anywidget | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.10%. Comparing base (12ca2f8) to head (b2cc3d0). Report is 155 commits behind head on main.

:exclamation: Current head b2cc3d0 differs from pull request most recent head 0fbb393

Please upload reports for the commit 0fbb393 to get more accurate results.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #598 +/- ## ========================================== - Coverage 98.45% 97.10% -1.35% ========================================== Files 8 9 +1 Lines 452 553 +101 ========================================== + Hits 445 537 +92 - Misses 7 16 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[kwentine]

Related to ease-of-testing (among other benefits) I throw in an idea: instead of using a long inline template, would it be worthwhile to extract it in a hmr.js file that assumes the existence of global.__anywidget_hmr_src, and just template a global.__anywidget_hmr_src=<src> header ? The concatenation of hmr.js and header being then served by the plugin.

[manzt]

Related to ease-of-testing (among other benefits) I throw in an idea: instead of using a long inline template, would it be worthwhile to extract it in a hmr.js file that assumes the existence of global.anywidget_hmr_src, and just template a global.anywidget_hmr_src= header ? The concatenation of hmr.js and header being then served by the plugin.

love the idea! Yes, the HMR stuff has just grown in complexity beyond an inline string. I think we could just find and replace a special string:

// hmr.js
const SRC = "__ANYWIDGET_HMR_SRC__";

import.meta.hot.accept(SRC, (newModule) => {
  // ...
});

export async function render({ model, el } ) {
  if (import.meta.hot.data.render == null) {
    let m = await import(SRC);
    // ...
  }
  // ...
}

And simply:

// vite/index.js
import * as fs from "node:fs";
import * as path from "node:path";
import * as url from "node:url";

// ceremony to get the filepath, in node>=v20 import.meta.dirname
let __dirname = path.dirname(url.fileURLToPath(import.meta.url));
let template = fs.readFileSync(path.resolve(__dirname, "hmr.js"), { encoding: "utf-8" });

then in load(id)

return template.replace("__ANYWIDGET_HMR_SRC__", id.split(":")[1]);

[kwentine]

So I gave it a try. Daunted by the file loading subtleties I relied on the esbuild file loader. And i seemed that Vite's static analysis was puzzled by the SRC = "__ANYWIDGET_HMR_SRC__" indirection, so I had to inline the assigned string repeatedly instead. Tested locally of course 😇

[manzt]

Daunted by the file loading subtleties I relied on the esbuild file loader.

Interesting workaround! Unfortunately, we only use esbuild to generate the .cjs version of the package (i.e., so folks can require("@anywidget/vite") rather than import from their vite.config.js. We ship index.js source directly to npm use in Node via ESM; that means it's the main entry point for the package and is not intended to be bundled.

The code snippet I shared should work:

import * as fs from "node:fs";
import * as path from "node:path";
import * as url from "node:url";

let __dirname = path.dirname(url.fileURLToPath(import.meta.url));
let template = fs.readFileSync(path.resolve(__dirname, "hmr.js"), { encoding: "utf-8" });

And i seemed that Vite's static analysis was puzzled by the SRC = "ANYWIDGET_HMR_SRC"

Ah, I guess that makes sense. Thanks for the fix!

[kwentine]

I adopted your suggestion, thanks for the clarification. I think my understanding of the matter is limited, though 🤔

I chose a different route in the first place because I now get these errors:

• A build time warning: "import.meta" is not available with the "cjs" output format and will be empty
• An error running vite serve from a test project: Identifier '__dirname' has already been declared

My vite.config.js uses import anywidget from '@anywidget/vite' and I have node_modules/@anywidget/vite -symlink-> path/to/local/repo/anywidget/packages/vite.

I have the impression that notwithstanding the import syntax, node loads the built dist/index.cjs. Hence the esbuild text loader approach worked for me although I did not require('@anywidget/vite') in vite.config.js.

[kwentine]

A little idea before the weekend: could we not trigger the builtin anywidget HMR mechanism in response to Vite HMR updates ? The following snippet could be injected by a transform plugin just before export default { render }

if (import.meta.hot) {

    let SRC = "/index.js"; // "__ANYWIDGET_HMR_SRC__"
    let PORT = "5173";     // "__ANYWIDGET_HMR_PORT__"

    // Touch _esm on all active models
    async function refresh() {
    for (let m of import.meta.hot.data.models) {
        m.set("_esm", `http://localhost:${PORT}${SRC}?t={Date.now()}`);
    }
    }

    let originalRender = render;
    render = async function ({model, el}) {
    import.meta.hot.data.models.add(model);
    return await originalRender({model, el})
    }

    // Register event handlers on first load only
    if (!import.meta.hot.data.models) {

    import.meta.hot.data.models = new Set();

    import.meta.hot.on('vite:beforeUpdate', (data) => {
        for (let u of data.updates) {
        if (u.path === SRC) {
            return refresh()
        }
        }
    })

    // Accept HMR update to avoid full page reload
    import.meta.hot.accept((newModule) => {})

    }    
}

Be that as it may, I wish you a nice weekend ;)

[manzt]

I have not forgotten about this! I've been busy working on #603 and just getting back to this. I'll try to give review and respond to your commends ASAP.

[manzt]

I adopted your suggestion, thanks for the clarification. I think my understanding of the matter is limited, though 🤔

I wonder if we can just go ESM-only. and forget the esbuild stuff. NodeJS supports require-ing ESM now, so I think we can move towards ESM-only plugin.

I have the impression that notwithstanding the import syntax, node loads the built dist/index.cjs. Hence the esbuild text loader approach worked for me although I did not require('@anywidget/vite') in vite.config.js.

Node loads the dist/index.cjs if "type": "module" is not set the package.json.

[manzt]

A little idea before the weekend: could we not trigger the builtin anywidget HMR mechanism in response to Vite HMR updates ?

The built-in anywidget HMR will not be enabled when using Vite. Built-in HMR only starts up if a file path is detected for _esm, and when you use vite you have a URL:

_esm = "http://localhost:5173/index.js?anywidget"

[manzt]

~Unfortunately, I'm getting a weird console error with the plugin now:~

TypeError: Failed to fetch dynamically imported module: http://localhost:5173/index.tsx?anywidget

~when I use a dynamic import with Vite.~

I spoke too soon! Seems to be working.

[manzt]

Thanks so much for the contribution! Will cut a release ASAP.

[kwentine]

Thanks for your advice and explanation in the process of getting this merged, it was fun and instructive. 👍🏻