astro icon indicating copy to clipboard operation
astro copied to clipboard

🐛 BUG: Using `preact/compat` with `solid-js` leads to "Cannot read properties of null (reading 'useRef')"

Open mayank99 opened this issue 3 years ago • 6 comments

What version of astro are you using?

1.0.0-rc.1

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

See linked stackblitz. Using client:load causes the solid-js adapter to mess with preact components. Using client:only makes the error go away.

I've also seen other variations of this issue, e.g. the react adapter sometimes messes with solid-js components. Will update this issue with more links (or create a new one) if I bump into that again.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-bltmyz?file=src%2Fpages%2Findex.astro

Participation

  • [ ] I am willing to submit a pull request for this issue.

mayank99 avatar Jul 29 '22 15:07 mayank99

I think this isn't directly related to Solid and is an issue with preact/compat and the downshift. Will report back after I investigate more.

matthewp avatar Aug 03 '22 21:08 matthewp

Made a little bit of progress here: https://github.com/withastro/astro/tree/preact-compat-issue

  • this commit: https://github.com/withastro/astro/commit/d453f976dbb09cc8371783403f4fed38ee944115

Going to chat with @bluwy about this. The issues I'm running into are:

  • SSR seems to transform node_modules/preact/compat/dist/compat.module.js incorrectly, I think because it contains an import statement and an export from statement about the same module.

Here is my forked stackblitz: https://stackblitz.com/edit/github-bltmyz-5rgtvb?file=astro.config.mjs


Steps to get to where I left off:

  1. Download the stackblitz project locally.
  2. npm install
  3. Add the changes from https://github.com/withastro/astro/commit/d453f976dbb09cc8371783403f4fed38ee944115 to your local node_modules/@astrojs/preact/dist/index.js
  4. npm start
  5. Should see some error about __vite_ssr_import_1__ or something. This is a bug in Vite (I think).
    • this bug needs to be fixed to proceed. I think it's related to the fact that the module imports and exports from the same module.
  6. node --inspect-brk node_modules/.bin/astro dev will get you to a debugger.

matthewp avatar Aug 04 '22 16:08 matthewp

Oh that's super interesting. I think I got confused by the original error message because it says @astrojs/solid-js.

Does preact/compat not have full compatibility with all react (<18) packages?

Also, if solid-js is eliminated, does that make this issue a duplicate of https://github.com/withastro/astro/issues/4107? Different package there, similar problem.

mayank99 avatar Aug 04 '22 16:08 mayank99

I'm sure that preact/compat is not compatible with all React packages, that's an impossible ask. But I don't yet know if its compatible with this one or not, there's an underlying bug we have to fix first.

#4107 could be the same issue, assigning it to myself until we figure this one out.

matthewp avatar Aug 04 '22 18:08 matthewp

I managed to trim down the issue to Vite's SSR transform, which incorrectly transforms:

export * from 'vue'
import {createApp} from 'vue'
export {createApp as bar}

to

const __vite_ssr_import_1__ = await __vite_ssr_import__("vue");
__vite_ssr_exportAll__(__vite_ssr_import_1__);
Object.defineProperty(__vite_ssr_exports__, "bar", { enumerable: true, configurable: true, get(){ return __vite_ssr_import_0__.createApp }});

Note the __vite_ssr_import_0__ does not exists. I'll try to send a fix in Vite.

bluwy avatar Aug 05 '22 07:08 bluwy

Note: This should be good when https://github.com/vitejs/vite/pull/9554 is released and the Vite version used by Astro is bumped (currently pinned). And also a combination of the fix in this branch.

bluwy avatar Aug 08 '22 07:08 bluwy