-
Notifications
You must be signed in to change notification settings - Fork 426
CDN assets support #2319
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
Open
indeyets
wants to merge
6
commits into
solidjs:main
Choose a base branch
from
indeyets:cdn-assets-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
CDN assets support #2319
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
5bb73d6
🐛 Keep the URL scheme in SSR asset paths
indeyets f88b04b
🧪 Add failing tests for the app base
indeyets 164f72d
🐛 Separate the app base from the asset base
indeyets 4ed1afc
🐛 Wrap the app base in slashes
indeyets e8d7431
🏷️ Publish the SERVER_BASE_URL type
indeyets 45001d8
✅ Test the app with a CDN asset base
indeyets File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@solidjs/start": patch | ||
| --- | ||
|
|
||
| Support a full URL in Vite's `base` so assets can be served from a CDN, completing the base URL prefixing from 2.0.3. Entry, stylesheet, modulepreload and serialized manifest paths no longer collapse `https://` into `https:/`, and server functions post to the path the app is mounted at (`server.baseURL`, else Vite's `base` when it is a plain path, else `/`) instead of the asset base. There is no CDN in development: Vite serves the assets itself, including the public directory, under the URL's path, while the app stays at the root. Root-relative links to public files therefore differ between dev and a build; setting the CDN `base` for production builds only avoids that. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import { defineConfig, devices } from "@playwright/test"; | ||
|
|
||
| export default defineConfig({ | ||
| testDir: "./src/e2e-base", | ||
| testMatch: "**/*.test.ts", | ||
|
|
||
| webServer: { | ||
| command: "pnpm run dev --config vite.config.base.ts --host 127.0.0.1 --port 3001 --strictPort", | ||
| url: "http://127.0.0.1:3001", | ||
| reuseExistingServer: true, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }, | ||
|
|
||
| use: { | ||
| baseURL: "http://127.0.0.1:3001", | ||
| trace: "on-first-retry", | ||
| }, | ||
|
|
||
| projects: [ | ||
| { | ||
| name: "chromium", | ||
| use: { ...devices["Desktop Chrome"] }, | ||
| }, | ||
| ], | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { expect, test } from "@playwright/test"; | ||
|
|
||
| test.describe("app with a CDN asset base", () => { | ||
| test("serves the page at the root and its entry script under the asset base", async ({ | ||
| page, | ||
| }) => { | ||
| const response = await page.goto("/"); | ||
|
|
||
| expect(response?.status()).toBe(200); | ||
| await expect(page.locator('script[type="module"][src]').first()).toHaveAttribute( | ||
| "src", | ||
| /^\/some\/prefix\//, | ||
| ); | ||
| }); | ||
|
|
||
| test("calls server functions at the root, not under the asset base", async ({ page }) => { | ||
| const serverFunctionCalls: string[] = []; | ||
| page.on("request", request => { | ||
| if (request.url().includes("_server")) serverFunctionCalls.push(request.url()); | ||
| }); | ||
|
|
||
| await page.goto("/is-server-nested"); | ||
|
|
||
| await expect(page.locator("#server-fn-test")).toContainText('{"serverFnWithIsServer":true}'); | ||
| expect(serverFunctionCalls.length).toBeGreaterThan(0); | ||
| for (const url of serverFunctionCalls) { | ||
| expect(new URL(url).pathname).toMatch(/^\/_server/); | ||
| } | ||
| }); | ||
|
|
||
| test("matches API routes at the root", async () => { | ||
| const response = await fetch("http://127.0.0.1:3001/api/text-plain"); | ||
|
|
||
| expect(await response.text()).toBe("test"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import { defineConfig, mergeConfig } from "vite"; | ||
| import config from "./vite.config.ts"; | ||
|
|
||
| // A CDN base, as a production build would set it, kept for dev on purpose: dev | ||
| // has no CDN, so Vite serves the assets itself under the URL's path, /some/prefix/, | ||
| // while the app stays at the root, as it would with the assets on the CDN. | ||
| export default mergeConfig(config, defineConfig({ base: "https://cdn.example.com/some/prefix/" })); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from "node:fs"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; | ||
| import type { ConfigEnv, Plugin, UserConfig } from "vite"; | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
|
|
||
| import { solidStart } from "./index.ts"; | ||
|
|
||
| let root: string; | ||
|
|
||
| beforeEach(() => { | ||
| root = realpathSync.native(mkdtempSync(join(tmpdir(), "solid-start-config-"))); | ||
| mkdirSync(join(root, "src/routes"), { recursive: true }); | ||
| writeFileSync(join(root, "src/app.tsx"), "export default () => null;"); | ||
| vi.spyOn(process, "cwd").mockReturnValue(root); | ||
| // the build branch of the config hook collects route entries from here | ||
| vi.stubGlobal("ROUTERS", { client: { getRoutes: async () => [] } }); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.restoreAllMocks(); | ||
| vi.unstubAllGlobals(); | ||
| rmSync(root, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| /** The `SERVER_BASE_URL` the `solid-start:config` plugin defines for a user config. */ | ||
| async function serverBaseUrl(config: UserConfig, command: ConfigEnv["command"] = "build") { | ||
| const plugin = solidStart() | ||
| .flat() | ||
| .find( | ||
| (p): p is Plugin => | ||
| !!p && typeof p === "object" && "name" in p && p.name === "solid-start:config", | ||
| ); | ||
| const hook = plugin!.config as (config: UserConfig, env: ConfigEnv) => Promise<UserConfig>; | ||
| const mode = command === "build" ? "production" : "development"; | ||
| const resolved = await hook(config, { command, mode }); | ||
|
|
||
| return JSON.parse(resolved.define!["import.meta.env.SERVER_BASE_URL"] as string) as string; | ||
| } | ||
|
|
||
| describe("SERVER_BASE_URL", () => { | ||
| const commands: ConfigEnv["command"][] = ["build", "serve"]; | ||
|
|
||
| it.each(commands)("defaults to the root (%s)", async command => { | ||
| await expect(serverBaseUrl({}, command)).resolves.toBe("/"); | ||
| }); | ||
|
|
||
| it.each(commands)("follows a path-only Vite base (%s)", async command => { | ||
| await expect(serverBaseUrl({ base: "/app/" }, command)).resolves.toBe("/app/"); | ||
| }); | ||
|
|
||
| it.each(commands)("adds the leading slash Vite adds (%s)", async command => { | ||
| await expect(serverBaseUrl({ base: "app/" }, command)).resolves.toBe("/app/"); | ||
| }); | ||
|
|
||
| it.each(commands)("maps a relative Vite base to the root (%s)", async command => { | ||
| await expect(serverBaseUrl({ base: "./" }, command)).resolves.toBe("/"); | ||
| await expect(serverBaseUrl({ base: "" }, command)).resolves.toBe("/"); | ||
| }); | ||
|
|
||
| it.each(commands)("maps an external Vite base to the root (%s)", async command => { | ||
| const cdn = { base: "https://cdn.example.com/some/prefix/" }; | ||
|
|
||
| await expect(serverBaseUrl(cdn, command)).resolves.toBe("/"); | ||
| await expect(serverBaseUrl({ base: "//cdn.example.com/" }, command)).resolves.toBe("/"); | ||
| }); | ||
|
|
||
| it("prefers an explicit server.baseURL", async () => { | ||
| const config = { base: "https://cdn.example.com/", server: { baseURL: "/app/" } } as UserConfig; | ||
|
|
||
| await expect(serverBaseUrl(config)).resolves.toBe("/app/"); | ||
| }); | ||
|
|
||
| it("wraps server.baseURL in slashes", async () => { | ||
| const config = { server: { baseURL: "app" } } as UserConfig; | ||
|
|
||
| await expect(serverBaseUrl(config)).resolves.toBe("/app/"); | ||
| }); | ||
|
|
||
| it.each(commands)("adds the trailing slash a Vite base may lack (%s)", async command => { | ||
| await expect(serverBaseUrl({ base: "/app" }, command)).resolves.toBe("/app/"); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure if this needs to handle an empty string, might not be an issue.