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

[Bug]: Solid-Start should not reuse Vite's "base" setting for the router #1132

Closed
2 tasks done
indeyets opened this issue Nov 28, 2023 · 7 comments
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@indeyets
Copy link
Contributor

indeyets commented Nov 28, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Vite's "base" setting, which is originally intended as the base-url for assets is reused for solid-router here: https://github.com/solidjs/solid-start/blame/main/packages/start/entry-client/StartClient.tsx#L98

current behaviour is originally defined by @atk and then fine-tuned by @devinrhode2

Expected behavior 🤔

Solid-Start should provide a separate independent setting for router's base-path to support cases when assets are served from one path, but actual app is served from another.

Steps to reproduce 🕹

Steps (sorry, its "fake" data, as I don't know how to make real-life reproduction for this):

  1. Set "base" setting in "vite.config.ts" to https://cdn.example.com/custom/path
  2. Deploy app to https://app.example.com/
  3. Open the page
  4. See that all of dynamic JS-based elements defined "inside" of router are NOT dynamic (hydration didn't run)

Context 🔦

My situation looks like this:

We serve assets (js, css, … files) from the CDN and use both custom host and custom path-prefix there. Vite's "base" setting is perfect for configuring this.

So, for example, it looks like: https://cdn.example.com/custom/path

Solid start reads this setting and reuses the path for the base setting of solid-router.

But the problem is, that the app is served from: https://app.example.com/ and base=/custom/path SILENTLY breaks the routing and while server-side renders the pages, client-side doesn't even try to hydrate them. No error or warning here.

Your environment 🌎

No response

@indeyets indeyets added the bug Something isn't working label Nov 28, 2023
@indeyets
Copy link
Contributor Author

indeyets commented Nov 28, 2023

my temporary local solution is this YARN patch:

diff --git a/entry-client/StartClient.tsx b/entry-client/StartClient.tsx
index a9b6ecb4bdb13d07ceae0d1246745ee527638b4f..e8d551f9b6e5609f3c20d830dafd9a4af99655a0 100644
--- a/entry-client/StartClient.tsx
+++ b/entry-client/StartClient.tsx
@@ -95,7 +95,7 @@ export default ({
     );
   }
 
-  const BASE_URL = import.meta.env.BASE_URL;
+  const BASE_URL = "/"; 
   let basePath = BASE_URL;
   if (BASE_URL.startsWith("http")) {
     try {

@devinrhode2
Copy link
Contributor

100% agree with you here. That would be the correct fix I maybe should have done originally. I suppose since SolidStart is still in beta, it's no problem to just make the change. But even if we wanted to avoid breaking apps, we could have a behavior like so: if NEW_SOLIDSTART_BASE_PATH is defined, use that for routing instead of vites BASE

@ryansolid
Copy link
Member

Part of the solution here in SolidStart beta 2 is putting the router outside of Start so you always configure the base path yourself. But yeah this came as a result of a PR so clearly people expected this behavior.

I'm debating how to handle it as I'm sort of at a stage where I'm not trying to do breaking changes in the current beta. And this is fixed in the next version.

@indeyets
Copy link
Contributor Author

indeyets commented Nov 29, 2023

oh… right.

@ryansolid I just figured out that it is possible to do several simple things:

  1. allow to pass base as the parameter to <StartClient>
  2. in cases when base is not specified, but import.meta.env.BASE_URL is customised still use it to maintain BC, but produce a warning which tells what's going on and suggesting to pass base={import.meta.env.BASE_URL} explicitly if this is actually the desired behaviour or pass base="/" otherwise.
  3. change the app template to use <StartClient base="/" />

I can build this patch a bit later if it sounds right

@devinrhode2
Copy link
Contributor

@indeyets are you able to use beta v2?

@indeyets
Copy link
Contributor Author

I didn't have time to look at it from practical PoV. I really should. I expect middlewares will break (and I have 4 or 5 of those)

@ryansolid
Copy link
Member

In setting up for SolidStart's next Beta Phase built on Nitro and Vinxi we are closing all PRs/Issues that will not be merged due to the system changing. If you feel your issue was closed by mistake. Feel free to re-open it after updating/testing against 0.4.x release. Thank you for your patience.

See #1139 for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants