Doesn't work with vite 5.3

[cranelee]

Describe the bug

It works fine with sveltKit1/vite4. But I encounter some problems when I upgrade to svelteKit2.

❌ Encountered error in src/lib/components/mobile/home/mobileHomeHeader.svelte Cannot read properties of undefined (reading 'watchFiles')

Reproduction

No response

[jycouet]

It seems to be a vite 5.3.1 issue, going back to vite 5.2.13 looks good. People are speaking about it here: https://discord.com/channels/1024421016405016718/1252330494088056855

[cranelee]

@jycouet thanks, it still not working. The server error is fragment can only take fragment documents. Found: undefined

[cranelee]

@jycouet It works, I didn't remove the letter ^ before version at first time. Thanks a lot.

[SeppahBaws]

after some investigating: the error seems to be coming from here: https://github.com/HoudiniGraphql/houdini/blob/6c5dbda72fb4b7c42dc85a3991e9705224dda080/packages/houdini-svelte/src/plugin/transforms/componentQuery.ts#L226

which throws this error:

TypeError: Cannot read properties of undefined (reading 'watchFiles')
  at Object.addWatchFile (file:///C:/dev/houdini/node_modules/.pnpm/vite@5.3.1_@types+node@18.11.15/node_modules/vite/dist/node/chunks/dep-BcXSligG.js:49663:21)
  at Object.addWatchFile [as dependency] (file:///C:/dev/houdini/node_modules/.pnpm/vite@5.3.1_@types+node@18.11.15/node_modules/vite/dist/node/chunks/dep-BcXSligG.js:49815:11)
  at Object.enter (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89150:26)
  at AsyncWalker.visit (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:88994:26)
  at AsyncWalker.visit (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89025:22)
  at async AsyncWalker.visit (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89025:11)
  at async AsyncWalker.visit (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89025:11)
  at async AsyncWalker.visit (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89019:20)
  at async asyncWalk (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89053:10)
  at async find_graphql (file:///C:/dev/houdini/packages/houdini-svelte/build/plugin-esm/index.js:89056:3)

(doing the testing on my svelte-5 branch because it's the only one which has been upgraded to vite 5 so far...)

[SeppahBaws]

more investigating: (and mainly writing this down so that I don't forget)

When houdini-svelte looks for inline queries, it walks the AST. In the walker's enter node, there is a function call to page.watch_file. https://github.com/HoudiniGraphql/houdini/blob/6c5dbda72fb4b7c42dc85a3991e9705224dda080/packages/houdini-svelte/src/plugin/transforms/componentQuery.ts#L243

This page.watch_file is filled in the plugin setup code, correctly according to the docs https://github.com/HoudiniGraphql/houdini/blob/6c5dbda72fb4b7c42dc85a3991e9705224dda080/packages/houdini/src/vite/houdini.ts#L241 Rollup docs (Vite adopts these): https://rollupjs.org/plugin-development/#plugin-context

By the time that the AST walker comes around to the call to page.watch_file, it goes into this code in Vite: https://github.com/vitejs/vite/blob/cafa7d56f35ab0d55da732f16aa39569f31c581e/packages/vite/src/node/server/pluginContainer.ts#L787-L794

Which then in turn goes to: https://github.com/vitejs/vite/blob/cafa7d56f35ab0d55da732f16aa39569f31c581e/packages/vite/src/node/server/pluginContainer.ts#L597-L605

where for some reason this._container is undefined, hence causing the Cannot read properties of undefined (reading 'watchFiles')` error.

Seems to be a side-effect of the Vite plugin infrastructure rewrite in 5.3.0 -> vitejs/vite#17288

[macmillen]

@SeppahBaws so is this an issue that needs to be fixed on Houdini's or Vite's side? And if Vite's side - is there already an open issue?

[SeppahBaws]

@SeppahBaws so is this an issue that needs to be fixed on Houdini's or Vite's side? And if Vite's side - is there already an open issue?

I'm not sure, I haven't worked with vite plugin development before, but since their release didn't mention any breaking changes I am inclined to think that it's something to be fixed on Vite's side?

I might open up an issue there anyways to ask for help with fixing this if we can't figure it out on our side...

We'll also look at adding houdini to their vite-ecosystem CI pipeline so that any breaking changes are detected before a release is pushed 🙂

[macmillen]

I might open up an issue there anyways to ask for help with fixing this if we can't figure it out on our side...

We'll also look at adding houdini to their vite-ecosystem CI pipeline so that any breaking changes are detected before a release is pushed 🙂

Sounds like a great idea, thank you 🙂

[antfu]

It seems an edge case bug that bring in by the side-effect for refactoring the plugin container to a class. While should be fixed by watch_file: this.addWatchFile.bind(this), I agree this is more like a regression on Vite side, as Rollup doesn't require doing this.

[SeppahBaws]

It seems an edge case bug that bring in by the side-effect for refactoring the plugin container to a class. While should be fixed by watch_file: this.addWatchFile.bind(this), I agree this is more like a regression on Vite side, as Rollup doesn't require doing this.

Ah thanks for the reply! I had a conversation in Discord with @dominikg who wasn't entirely sure if it's a good idea to store the addWatchFile function since it is context sensitive. @antfu what's your insight into this?

[antfu]

After a brief discussion with the team, I think we made the consensus that this is considered an implementation detail, moving back would require us to do a lot manually .bind which would be costly on performance. In that sense, we will probably would not fix it. Also a bit context here (https://github.com/vitejs/vite/pull/15610#issuecomment-1893429121). In the meantime, we will try to improve our docs to make this clear.

I would suggest this plugin to:

• Use watch_file: this.addWatchFile.bind(this) for a quick fix
• In the long term, consider refactoring your code to avoid saving addWatchFile function to global as it's contextual, and always prefer using this.addWatchFile with the current context.