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

support "renderAssetUrl" and customized "assetsBase": "base" vite setting breaks solid-start build #114

Open
indeyets opened this issue Jan 10, 2024 · 23 comments
Labels
enhancement New feature or request

Comments

@indeyets
Copy link
Contributor

I need to be able to pass "base:" setting to vite to be able to prepend it to asset paths.
I don't need it to affect solid app, router or anything else. Just assets, like raw vite does it.

I used it with solid-start 0.3 using a custom patch to detach this setting from router.
I started to port app to 0.4 and current versions of solid-start disable babel transformations as soon as I specify base: "http://somehost.com" in vite config which is double weird.

@indeyets
Copy link
Contributor Author

So, strictly speaking, there are 2 distinct issues here:

  1. babel transformations should not break because config.base is specified
  2. config.base should be used as a prefix for generated urls of assets (but asset files should still be located in the usual location).

Example:

{
  base: 'https://cdn.example.com'
}

Files are still in .output/public/assets/*, all of the pages reference them as https://cdn.example.com/assets/*

@nksaraf
Copy link
Owner

nksaraf commented Jan 11, 2024

ill explain how things currently work and we will figure out how to fit this use case.

Vinxi is composed of sub routers, each of which have its own vite config, vite dev server, and nitro handler. We use base internally to set the sub path of the router. Eg. public static router's base is /, ssr router's base is also /, client router's base is /build and server functions route is /_server. That is why base is removed from the typescript types for the config.

Now for the use case where the whole app is under a sub path of a domain, eg. https://example.com/app/. Then you can set server.baseURL to /app to set the additional base for all the routers.

Now I imagine you want to set the base path to a certain domain. Is this both for dev and build? Is the whole app hosted at the cdn, or the server is somewhere else and the assets are somewhere else.

Vite's base property as per my understanding doesn't just affect the assets. It affects the whole app. Everything is served from that base. That's the behaviour I am expecting and what I implemented.

@nksaraf
Copy link
Owner

nksaraf commented Jan 11, 2024

I think we need to support two things:

  1. Domain name in the server.baseURL if its important to have a certain domain name in the URL. It will not be used in development, but should be used during the build.
  2. https://vitejs.dev/guide/build.html#advanced-base-options renderAssetUrl if you want to render certain URL's when you have different caching strategies for different assets. (remember we also have a server app that we need to care about)

@indeyets
Copy link
Contributor Author

I usually have separate configs for dev-mode (and dev-mode doesn't use "base:") and prod-mode (which does have "base:").

The app in production is served on one domain and doesn't have any prefix, but assets (css, js, images, fonts…) are served from a separate host, sometimes also with custom path-prefix.

Vite, originally, doesn't really have a concept of app. I don't think it is correct to imply that "base" should affect app's router. Those are really different concepts. Router is a dynamic thing and it can be configured during runtime (I, usually, have a separate app-config, which is read during startup and can change this behavior)
The matter of assets is a bit more complicated. I don't think there are ways to configure assets-prefix during runtime in vite. And, anyway, for my usecases I'm fine if it's only during the build time

@nksaraf
Copy link
Owner

nksaraf commented Jan 11, 2024

renderAssetUrl might be the best way to do this in that case. server.baseURL sets the app server's base url. vite's base we internally use (if you used raw vinxi you could set that yourself).

But if you want different urls for assets and the rest to be served at a common base, then renderAssetUrl is build for exactly this use case. It allows to design the URLs for your assets.

@indeyets
Copy link
Contributor Author

indeyets commented Jan 11, 2024

ok… I tried it. It works for some cases, but not for all of them. I guess this is exactly why I didn't use it originally.

{
  experimental: {
    renderBuiltUrl: (filename) => {
      return `http://example.com${filename}`;
    },
  },
}
  • CSS: <link href="/_build/assets/index-4f321e77.css" rel="stylesheet" precendence="high"/> — unprefixed 🟥
  • JS Preload: <link href="/_build/assets/index-09e36bf3.js" rel="modulepreload"/> — unprefixed 🟥
  • JS module: <script type="module" async src="/_build/assets/client-34bf29ad.js"> — unprefixed 🟥
  • Imported assets (images, etc.): http://example.comassets/anonymous_user@4-71b9adc3.png — prefixed 🟩 (I will add missing slash here, that's ok)

@indeyets
Copy link
Contributor Author

Looking at "Solid SSR" example in Readme

it looks like ability to inject custom host in base: before the /_build might solve my case exactly. It won't be full flexibility, but will be enough for me

@indeyets
Copy link
Contributor Author

indeyets commented Jan 12, 2024

I tried to replace base: in a specific vinxi-router of solid-start. It almost worked, but it was further prefixed with a /. So, the result was: /http://example.com/_build/…. I wonder where does that come from 🤔

https://github.com/solidjs/solid-start/blob/1daa2072b44ada5c8a20235aaf8ccb171f580505/packages/start/config/index.js#L179

@edivados
Copy link
Contributor

If this is about prod there is a good chance this here.

href: joinURL(app.config.server.baseURL ?? "/", router.base, asset),

@indeyets
Copy link
Contributor Author

@edivados thank you, but it also doesn't work properly.

If I remove app.config.server.baseURL ?? "/" part here I get prefect URLs in the web-pages, but actual css and js files are moved to .output/public/http:/example.com/_build/… 🤦

I need to detach file placement from url-generation

@indeyets
Copy link
Contributor Author

oh… right

next step:

  1. do not touch solid-start's config (leaving base: '/_build' there)
  2. change prod-server-manifest.js to:
href: joinURL('http://example.com/_build', asset),
key: join('http://example.com/_build', asset),
  • link to css: good 🟩
  • js preload: good 🟩
  • js module: no prefix 🟥

@edivados
Copy link
Contributor

If you look at build.js router.base is used all over the place for router build output, nitro handlers etc. so changing that to a URL would not work.

Maybe there is a need for a baseURL per router? I don't know what a good solution would look like.

@indeyets
Copy link
Contributor Author

@edivados more like optional assetsURLPrefix or even even a custom renderAssetUrl function, but the one which is properly applied to all of the static file urls.

@nksaraf
Copy link
Owner

nksaraf commented Jan 12, 2024

@edivados more like optional assetsURLPrefix or even even a custom renderAssetUrl function, but the one which is properly applied to all of the static file urls.

This solution with custom renderAssetUrl is probably the best.. but need it at runtime somehow .. so some assets are handled by vite some by Vinxi

That's what creates the issues. When you import in a entry-server it gets added as a server asset and so is in the /assets directory

Client assets are put in /_build/assets

@indeyets
Copy link
Contributor Author

Client assets are all I need now, I believe

@indeyets
Copy link
Contributor Author

@nksaraf The following local patch fixes the problem for me. Maybe I miss something (I did only initial testing), but seems to cover all of the important stuff.

@@ -0,0 +1,41 @@
diff --git a/lib/manifest/prod-server-manifest.js b/lib/manifest/prod-server-manifest.js
index 5ad9024b69f01daeabd78179097be85cdb473a88..d4bf36504b80d3cace55ba83b10b415dacdeba82 100644
--- a/lib/manifest/prod-server-manifest.js
+++ b/lib/manifest/prod-server-manifest.js
@@ -1,6 +1,7 @@
 import { joinURL } from "ufo";
 import invariant from "vinxi/lib/invariant";
 import { handlerModule, join, virtualId } from "vinxi/lib/path";
+import config from "config";
 
 import { pathToFileURL } from "node:url";
 
@@ -14,8 +15,8 @@ function createHtmlTagsForAssets(router, app, assets) {
 		.map((asset) => ({
 			tag: "link",
 			attrs: {
-				href: joinURL(app.config.server.baseURL ?? "/", router.base, asset),
-				key: join(app.config.server.baseURL ?? "", router.base, asset),
+				href: joinURL(config.urls.assets ?? "/", router.base, asset),
+				key: join(config.urls.assets ?? "", router.base, asset),
 				...(asset.endsWith(".css")
 					? { rel: "stylesheet", precendence: "high" }
 					: { rel: "modulepreload" }),
@@ -63,7 +64,7 @@ export function createProdManifest(app) {
 						let json = {};
 						for (const input of Object.keys(this.inputs)) {
 							json[input] = {
-								output: this.inputs[input].output.path,
+								output: joinURL(config.urls.assets, this.inputs[input].output.path),
 								assets: await this.inputs[input].assets(),
 							};
 						}
@@ -158,7 +159,7 @@ export function createProdManifest(app) {
 										},
 										output: {
 											path: joinURL(
-												app.config.server.baseURL ?? "",
+												config.urls.assets ?? "",
 												router.base,
 												bundlerManifest[id].file,
 											),

I cheated by using "config" to pass url-prefix. It would be great to be able to do the same without cheating, by passing something via the solid-start's vite.config

@nksaraf
Copy link
Owner

nksaraf commented Jan 15, 2024

How does config get here ? And are we sure this covers all the edge cases.. does it handle the assets that vite creates urls for

@indeyets
Copy link
Contributor Author

Config reads data from config files in my project. It's a singleton.

Yes, it works for imported images/binaries, if that's what you mean

@nksaraf
Copy link
Owner

nksaraf commented Jan 15, 2024

Yes, it works for imported images/binaries, if that's what you mean

I am surprised how. We haven't done anything to affect those urls in this changeset. Is there something else also changed.

@indeyets
Copy link
Contributor Author

@nksaraf sorry, I wasn't clear. The solution for "imported" assets is to use the function, as you suggested above. I still have that in my config. The patch is needed to cover the rest cases

#114 (comment)

@nksaraf
Copy link
Owner

nksaraf commented Jan 19, 2024

maybe we should call your renderAssetUrl function during build and inspect what changes are being done to the url and apply those to the url being built by the prod manifest.

@nksaraf nksaraf added the enhancement New feature or request label Jan 28, 2024
@nksaraf nksaraf changed the title "base" vite setting breaks solid-start build support "renderAssetUrl" and customized "assetsBase": "base" vite setting breaks solid-start build Jan 28, 2024
@birkskyum
Copy link
Contributor

birkskyum commented Mar 2, 2024

I'm a bit confused by the origin being in the base here. The way vite usually works is that you can set a vite.base which takes a path, but not an origin, which is enforced by requiring the base to start with a /. This warning is also printed by vinxi if you have an origin in the server.baseURL.

For advanced bases there's the experimental.renderBuiltUrl, which can have an origin.

For setting the origin (https://cdn.example.com) on dev server vite has a separate setting vite.server.origin.

@paularmstrong
Copy link

In my experience, using a separate host as a CDN for assets is very common – and I'm currently having issues with basically every hosting platform not keeping built assets around from one deploy to the next, so users are feeling a lot of pain points every time we deploy.

maybe we should call your renderAssetUrl function during build and inspect what changes are being done to the url and apply those to the url being built by the prod manifest.

This seems like a great option. I would love to get something going here – are there any blockers to implementing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants