Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(vike-solid-query): fix No QueryClient set & <QueryBoundary> #127

Merged
merged 4 commits into from
Sep 29, 2024

Conversation

phonzammi
Copy link
Member

@phonzammi phonzammi commented Sep 25, 2024

fix #123
fix #124

Regarding the <QueryBoundary> bug, I think it looks good to me in this commit.

The current release of vike-solid-query is compatible with examples/solid-query only within the vike-solid repository, and not for use outside of it.

No QueryClient set bug

I've been working to make it functional for both use cases and have identified two potential approaches, but neither is perfect:

  1. First approach : build Wrapper.tsx to Wrapper.jsx using tsc
    This works for both examples/solid-query and other use cases, but there is a warning :
Cannot optimize dependency: vike-solid-query/__internal/integration/Wrapper, present in 'optimizeDeps.include'

Please try pre-released as : 0.0.1-commit-21483db

  1. The second approach : Build everything using rollup

Both is wip, I need more try, and I need help 🙏

I believe it should be fixed by adding these export conditions in the package.json:

"./integration/Wrapper": {
  "solid": "./dist/integration/Wrapper.js", // this one is the fix
  "import": {
    "types": "./dist/integration/Wrapper.d.ts",
    "default": "./dist/integration/Wrapper.js"
  }
}

@phonzammi phonzammi force-pushed the phonzammi/fix-123 branch 2 times, most recently from 483edf8 to 5dc3c02 Compare September 25, 2024 07:02
@rtritto
Copy link
Contributor

rtritto commented Sep 25, 2024

I did a try installing the 0.0.1-commit-21483db version but always the 0.0.1 version is used:

// yarn.lock

- "vike-solid-query@npm:^0.0.1":
+ "vike-solid-query@npm:^0.0.1-commit-21483db":
  version: 0.0.1
  resolution: "vike-solid-query@npm:0.0.1"
  peerDependencies:
    "@tanstack/solid-query": ">=5.0.0"
    solid-js: ^1.8.7
    vike-solid: ">=0.7.3"
  checksum: 10c0/bb3c50b022933d845096619351b659544eb8fa79fd8b905a0363228f32d9bf2362bf82332f488b12dfcb91b5ede3f04dbfb164aa0238cf916eab3b10aa32d6bb
  languageName: node
  linkType: hard

@brillout
Copy link
Member

@rtritto That is a Yarn question. Have a look at and please respect https://brillout.github.io/rules/ (I believe I sent you this already). Thanks!

@rtritto
Copy link
Contributor

rtritto commented Sep 25, 2024

@rtritto That is a Yarn question. Have a look at and please respect https://brillout.github.io/rules/ (I believe I sent you this already). Thanks!

My bad, I correctly installed 0.0.1-commit-21483db and the issue has been fixed, thanks.

@phonzammi
Copy link
Member Author

  1. The second approach : Build everything using rollup, commit 01e0751

This approach works outside of vike-solid repo, the examples/solid-query will throws errors:
@brillout, Please take a look at this error

vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
vvvvvv ERROR & WARNING LOGS vvvvvv
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
[13:06:10.962][\examples\solid-query][pnpm run dev][stderr] 1:06:10 PM [vike@0.4.196][config][Bug] You stumbled upon a Vike bug. Go to https://github.com/vikejs/vike/issues/new and copy-paste this error. A maintainer will fix the bug (usually under 24 hours).
    at resolvePointerImport (file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected][email protected]_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig/resolvePointerImport.js:43:9)
    at resolvePointerImportOfConfig (file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected][email protected]_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig/resolvePointerImport.js:17:22)
    at getConfigValueSource (file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected][email protected]_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.js:576:[31](https://github.com/vikejs/vike-solid/actions/runs/11033624719/job/30645205771?pr=127#step:8:32))
    at file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected][email protected]_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.js:507:77
    at Array.map (<anonymous>)
    at resolveConfigValueSources (file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected]_terser@5.[32](https://github.com/vikejs/vike-solid/actions/runs/11033624719/job/30645205771?pr=127#step:8:33).0_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.js:507:51)
    at file:///D:/a/vike-solid/vike-solid/node_modules/.pnpm/[email protected][email protected]_@[email protected][email protected]_/node_modules/vike/dist/esm/node/plugin/plugins/importUserCode/v1-design/getVikeConfig.js:267:35
    at Array.map (<anonymous>)

@rtritto, also try this pre-released as 0.0.1-commit-01e0751

@rtritto
Copy link
Contributor

rtritto commented Sep 25, 2024

With 0.0.1-commit-01e0751, at Vike start:

3:35:59 PM [vike][config][Wrong Usage] The import import:./Wrapper.js:default defined in vike-solid-query/config couldn't be resolved: does './Wrapper.js' point to an existing file?

@phonzammi
Copy link
Member Author

Have you tried cleaning your repo first?

@rtritto
Copy link
Contributor

rtritto commented Sep 25, 2024

Have you tried cleaning your repo first?

Yes

@phonzammi
Copy link
Member Author

Hmm, I tested your repo (https://github.com/rtritto/template-vike-elysia-solid-daisyui/tree/test-solid-query), and it seems like it doesn't work with bun --bun vite, but it does work with vite.

@brillout
Copy link
Member

@phonzammi Thank you for the ping and, indeed, I could reproduce. I'll fix it later today.

Hmm, I tested your repo (https://github.com/rtritto/template-vike-elysia-solid-daisyui/tree/test-solid-query), and it seems like it doesn't work with bun --bun vite, but it does work with vite.

@rtritto Please double, triple, quadruple check whether it's a Vike issue or not before pinging us. It makes a big difference for us. As you can imagine, we've a lot of things to do, so anything that saves us time is a big help for us. Thank you! Looking forward to your future issues 👀

brillout added a commit to vikejs/vike that referenced this pull request Sep 25, 2024
@brillout
Copy link
Member

@phonzammi Thank you for the ping and, indeed, I could reproduce. I'll fix it later today.

Done (vikejs/vike@11810cf). You'll get a proper error message in the next release. You cannot use relative imports like you're trying (because of a Vite limitation).

@phonzammi phonzammi marked this pull request as ready for review September 27, 2024 17:03
@phonzammi
Copy link
Member Author

I think it should be fixed now.

@rtritto, please try this Pre-released as 0.0.1-commit-6abb0a2

@rtritto
Copy link
Contributor

rtritto commented Sep 27, 2024

With 0.0.1-commit-6abb0a2, I have no errors on imports (Bun and Node.js).
Thanks!

@phonzammi
Copy link
Member Author

We're glad to know it works now!

LGTM then 🚀

@phonzammi phonzammi requested a review from magne4000 September 27, 2024 18:13
@magne4000
Copy link
Member

I'm doing the review tomorrow

@magne4000
Copy link
Member

LGTM.

solid plugin bundling is really not on point. I would prefer that they would have official support for esbuild, and auto target generation for ssr/non-ssr files.

@magne4000 magne4000 merged commit ed0209e into main Sep 29, 2024
5 checks passed
@magne4000 magne4000 deleted the phonzammi/fix-123 branch September 29, 2024 09:07
@magne4000
Copy link
Member

@phonzammi You have to publish the new release, I do not have the accesses on npm for vike-solid-query

@magne4000
Copy link
Member

Also, publish it as 0.1.0 as every 0.0.x version is considered a breaking change in semver.

@phonzammi
Copy link
Member Author

Thank you.

Released as 0.1.0

@magne4000
Copy link
Member

solid plugin bundling is really not on point. I would prefer that they would have official support for esbuild, and auto target generation for ssr/non-ssr files.

@phonzammi just pointed out to me that https://github.com/solidjs-community/tsup-preset-solid exists. We need to try it!

@rtritto
Copy link
Contributor

rtritto commented Sep 29, 2024

@phonzammi just pointed out to me that https://github.com/solidjs-community/tsup-preset-solid exists. We need to try it!

Agree, tsup should be fast, powerful and simple.

Yesterday, I started (discovered) using it and I did elysiajs/node-adapter#4 (maybe can help).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants