From 3c72a317d7e7bdc9d029d199387ec16898dd37ec Mon Sep 17 00:00:00 2001 From: Okiki Date: Mon, 20 May 2024 04:30:47 +0000 Subject: [PATCH 1/9] fix: bandwidth issues for 404 resources #6302 Signed-off-by: Okiki --- packages/qwik-city/middleware/azure-swa/index.ts | 9 ++++++++- packages/qwik-city/middleware/bun/index.ts | 8 +++++++- .../qwik-city/middleware/cloudflare-pages/index.ts | 8 +++++++- packages/qwik-city/middleware/deno/index.ts | 8 +++++++- .../qwik-city/middleware/netlify-edge/index.ts | 8 +++++++- packages/qwik-city/middleware/node/index.ts | 12 +++++++++--- .../request-handler/generated/not-found-paths.ts | 4 ++++ packages/qwik-city/middleware/vercel-edge/index.ts | 8 +++++++- starters/e2e/qwikcity/adapter.spec.ts | 14 ++++++++++++++ 9 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 starters/e2e/qwikcity/adapter.spec.ts diff --git a/packages/qwik-city/middleware/azure-swa/index.ts b/packages/qwik-city/middleware/azure-swa/index.ts index 507b355dd4c..000cdfd4630 100644 --- a/packages/qwik-city/middleware/azure-swa/index.ts +++ b/packages/qwik-city/middleware/azure-swa/index.ts @@ -8,6 +8,7 @@ import type { import { getNotFound } from '@qwik-city-not-found-paths'; import { _deserializeData, _serializeData, _verifySerializable } from '@builder.io/qwik'; import { parseString } from 'set-cookie-parser'; +import { isStaticPath } from '@qwik-city-static-paths'; // @builder.io/qwik-city/middleware/azure-swa @@ -123,7 +124,13 @@ export function createQwikCity(opts: QwikCityAzureOptions): AzureFunction { // qwik city did not have a route for this request // response with 404 for this pathname - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(req.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/packages/qwik-city/middleware/bun/index.ts b/packages/qwik-city/middleware/bun/index.ts index 4f44f9134d8..3cc057523b4 100644 --- a/packages/qwik-city/middleware/bun/index.ts +++ b/packages/qwik-city/middleware/bun/index.ts @@ -101,7 +101,13 @@ export function createQwikCity(opts: QwikCityBunOptions) { const notFound = async (request: Request) => { try { const url = new URL(request.url); - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(request.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return new Response(notFoundHtml, { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/packages/qwik-city/middleware/cloudflare-pages/index.ts b/packages/qwik-city/middleware/cloudflare-pages/index.ts index 156fbfa4443..22e3d04047a 100644 --- a/packages/qwik-city/middleware/cloudflare-pages/index.ts +++ b/packages/qwik-city/middleware/cloudflare-pages/index.ts @@ -114,7 +114,13 @@ export function createQwikCity(opts: QwikCityCloudflarePagesOptions) { // qwik city did not have a route for this request // response with 404 for this pathname - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(request.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return new Response(notFoundHtml, { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/packages/qwik-city/middleware/deno/index.ts b/packages/qwik-city/middleware/deno/index.ts index b89e749806c..4afd9cff5fd 100644 --- a/packages/qwik-city/middleware/deno/index.ts +++ b/packages/qwik-city/middleware/deno/index.ts @@ -102,7 +102,13 @@ export function createQwikCity(opts: QwikCityDenoOptions) { const notFound = async (request: Request) => { try { const url = new URL(request.url); - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(request.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return new Response(notFoundHtml, { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/packages/qwik-city/middleware/netlify-edge/index.ts b/packages/qwik-city/middleware/netlify-edge/index.ts index 5edee2727ab..703cb6af86e 100644 --- a/packages/qwik-city/middleware/netlify-edge/index.ts +++ b/packages/qwik-city/middleware/netlify-edge/index.ts @@ -75,7 +75,13 @@ export function createQwikCity(opts: QwikCityNetlifyOptions) { // qwik city did not have a route for this request // response with 404 for this pathname - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(request.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return new Response(notFoundHtml, { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/packages/qwik-city/middleware/node/index.ts b/packages/qwik-city/middleware/node/index.ts index 51521406e50..1e53e714d31 100644 --- a/packages/qwik-city/middleware/node/index.ts +++ b/packages/qwik-city/middleware/node/index.ts @@ -74,7 +74,13 @@ export function createQwikCity(opts: QwikCityNodeRequestOptions) { if (!res.headersSent) { const origin = computeOrigin(req, opts); const url = getUrl(req, origin); - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(req.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); res.writeHead(404, { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname, @@ -105,8 +111,9 @@ export function createQwikCity(opts: QwikCityNodeRequestOptions) { } else { filePath = join(staticFolder, pathname, 'index.html'); } - const stream = createReadStream(filePath); const ext = extname(filePath).replace(/^\./, ''); + const stream = createReadStream(filePath); + stream.on('error', next); const contentType = MIME_TYPES[ext]; @@ -118,7 +125,6 @@ export function createQwikCity(opts: QwikCityNodeRequestOptions) { res.setHeader('Cache-Control', opts.static.cacheControl); } - stream.on('error', next); stream.pipe(res); return; diff --git a/packages/qwik-city/middleware/request-handler/generated/not-found-paths.ts b/packages/qwik-city/middleware/request-handler/generated/not-found-paths.ts index 8a59dcff065..e89494eefca 100644 --- a/packages/qwik-city/middleware/request-handler/generated/not-found-paths.ts +++ b/packages/qwik-city/middleware/request-handler/generated/not-found-paths.ts @@ -1,3 +1,7 @@ +/** + * DO NOT CHANGE THE EXACT VALUE OF `Resource Not Found`, THIS WILL BE REPLACED IN THE CODE LATER, + * DEPENDING ON RUNTIME (e.g. DEV or PRODUCTION) + */ export function getNotFound(_pathname: string) { return 'Resource Not Found'; } diff --git a/packages/qwik-city/middleware/vercel-edge/index.ts b/packages/qwik-city/middleware/vercel-edge/index.ts index 52cc64ed608..088010e0087 100644 --- a/packages/qwik-city/middleware/vercel-edge/index.ts +++ b/packages/qwik-city/middleware/vercel-edge/index.ts @@ -102,7 +102,13 @@ export function createQwikCity(opts: QwikCityVercelEdgeOptions) { // qwik city did not have a route for this request // response with 404 for this pathname - const notFoundHtml = getNotFound(url.pathname); + + // In the development server, we replace the getNotFound function + // For static paths, we assign a static "Not Found" message. + // This ensures consistency between development and production environments for specific URLs. + const notFoundHtml = isStaticPath(request.method || 'GET', url) + ? 'Not Found' + : getNotFound(url.pathname); return new Response(notFoundHtml, { status: 404, headers: { 'Content-Type': 'text/html; charset=utf-8', 'X-Not-Found': url.pathname }, diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts new file mode 100644 index 00000000000..bc43b7f01e7 --- /dev/null +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -0,0 +1,14 @@ +import { expect, test } from "@playwright/test"; + +test.describe("Qwik City Adapter", () => { + test("Qwik City Adapter", async ({ page: api }) => { + const rsp = (await api.goto("/qwikcity-test/build/a-random-file-after-that.js"))!; + expect(rsp.status()).toBe(200); + expect(rsp.headers()["content-type"]).toBe( + "text/html; charset=utf-8", + ); + + const data = await rsp.text(); + expect(data).toBe("Not Found"); + }); +}); From e2b9171c3bafa1650bd2e1a447dc4cfd3934d47a Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 21:54:57 -0700 Subject: [PATCH 2/9] style: lint --- starters/e2e/qwikcity/adapter.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index bc43b7f01e7..e08e8a2f4aa 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -2,11 +2,11 @@ import { expect, test } from "@playwright/test"; test.describe("Qwik City Adapter", () => { test("Qwik City Adapter", async ({ page: api }) => { - const rsp = (await api.goto("/qwikcity-test/build/a-random-file-after-that.js"))!; + const rsp = (await api.goto( + "/qwikcity-test/build/a-random-file-after-that.js", + ))!; expect(rsp.status()).toBe(200); - expect(rsp.headers()["content-type"]).toBe( - "text/html; charset=utf-8", - ); + expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); const data = await rsp.text(); expect(data).toBe("Not Found"); From 2964700a31833012ba562d3b8482aee07d680969 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 22:40:36 -0700 Subject: [PATCH 3/9] chore: fix no tetst --- .../request-handler/generated/static-paths.ts | 19 +++++++++++++++++++ starters/e2e/qwikcity/adapter.spec.ts | 14 -------------- 2 files changed, 19 insertions(+), 14 deletions(-) delete mode 100644 starters/e2e/qwikcity/adapter.spec.ts diff --git a/packages/qwik-city/middleware/request-handler/generated/static-paths.ts b/packages/qwik-city/middleware/request-handler/generated/static-paths.ts index c1fe86d3932..3ad2251c8bc 100644 --- a/packages/qwik-city/middleware/request-handler/generated/static-paths.ts +++ b/packages/qwik-city/middleware/request-handler/generated/static-paths.ts @@ -12,5 +12,24 @@ export function isStaticPath(method: string, url: URL) { if (url.search !== '') { return false; } + + /** + * - Generated values in vite post build + * + * Const p = url.pathname; + * + * If (p.startsWith(baseBuildPath)) { return true; } + * + * If (p.startsWith(assetsPath)) { return true; } + * + * If (staticPaths.has(p)) { return true; } + * + * If (p.endsWith('/q-data.json')) { const pWithoutQdata = p.replace(//q-data.json$/, ''); + * + * If (staticPaths.has(pWithoutQdata + '/')) { return true; } + * + * If (staticPaths.has(pWithoutQdata)) { return true; } + */ + return /\.(jpg|jpeg|png|webp|avif|gif|svg)$/.test(url.pathname); } diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts deleted file mode 100644 index e08e8a2f4aa..00000000000 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { expect, test } from "@playwright/test"; - -test.describe("Qwik City Adapter", () => { - test("Qwik City Adapter", async ({ page: api }) => { - const rsp = (await api.goto( - "/qwikcity-test/build/a-random-file-after-that.js", - ))!; - expect(rsp.status()).toBe(200); - expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); - - const data = await rsp.text(); - expect(data).toBe("Not Found"); - }); -}); From 85a22c3154b78b43209f3b3640d90b5feb458b29 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 22:53:09 -0700 Subject: [PATCH 4/9] chore: add not found in tests --- starters/dev-server.ts | 14 +++++++++++++- starters/e2e/qwikcity/adapter.spec.ts | 14 ++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 starters/e2e/qwikcity/adapter.spec.ts diff --git a/starters/dev-server.ts b/starters/dev-server.ts index e583b256b21..dd62a9f69e4 100644 --- a/starters/dev-server.ts +++ b/starters/dev-server.ts @@ -148,7 +148,19 @@ export { `; } if (id.endsWith(qwikCityStaticPaths)) { - return `export function isStaticPath(){ return false; };`; + return `export function isStaticPath(method, url){ + if (method !== 'GET') { + return false; + } + if (url.search !== '') { + return false; + } + const p = url.pathname; + if (p.includes('/build/')) { + return true; + } + return false; + };`; } if (id.endsWith(qwikCityNotFoundPaths)) { const notFoundHtml = getErrorHtml(404, "Resource Not Found"); diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts new file mode 100644 index 00000000000..e08e8a2f4aa --- /dev/null +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -0,0 +1,14 @@ +import { expect, test } from "@playwright/test"; + +test.describe("Qwik City Adapter", () => { + test("Qwik City Adapter", async ({ page: api }) => { + const rsp = (await api.goto( + "/qwikcity-test/build/a-random-file-after-that.js", + ))!; + expect(rsp.status()).toBe(200); + expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); + + const data = await rsp.text(); + expect(data).toBe("Not Found"); + }); +}); From ca673a08503a083c9fad6a350b85a084ef038d71 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 22:54:45 -0700 Subject: [PATCH 5/9] chore: test --- starters/e2e/qwikcity/adapter.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index e08e8a2f4aa..2d57231d17d 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -10,5 +10,7 @@ test.describe("Qwik City Adapter", () => { const data = await rsp.text(); expect(data).toBe("Not Found"); + // "Resource Not Found" is replaced in the development server + expect(data).not.toBe("Resource Not Found"); }); }); From 46aed289993352467153cfe8c12d5ce4d7654995 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 23:35:17 -0700 Subject: [PATCH 6/9] chore: maybe fix --- starters/e2e/qwikcity/adapter.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index 2d57231d17d..3438a94791a 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -3,7 +3,7 @@ import { expect, test } from "@playwright/test"; test.describe("Qwik City Adapter", () => { test("Qwik City Adapter", async ({ page: api }) => { const rsp = (await api.goto( - "/qwikcity-test/build/a-random-file-after-that.js", + "/qwikcity-test/adapter/build/a-random-file-after-that.js", ))!; expect(rsp.status()).toBe(200); expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); From cb16f4d3ba72218f7a944c300aac56820dcf0368 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 23:36:19 -0700 Subject: [PATCH 7/9] fix: status --- starters/e2e/qwikcity/adapter.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index 3438a94791a..4ae9a035ff4 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -5,7 +5,7 @@ test.describe("Qwik City Adapter", () => { const rsp = (await api.goto( "/qwikcity-test/adapter/build/a-random-file-after-that.js", ))!; - expect(rsp.status()).toBe(200); + expect(rsp.status()).toBe(404); expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); const data = await rsp.text(); From 112ef30d25b850f2db88377cbeafeeff5392ebe1 Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Sun, 19 May 2024 23:57:49 -0700 Subject: [PATCH 8/9] chore: fix --- starters/dev-server.ts | 14 +------------- starters/e2e/qwikcity/adapter.spec.ts | 3 ++- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/starters/dev-server.ts b/starters/dev-server.ts index dd62a9f69e4..379da438f88 100644 --- a/starters/dev-server.ts +++ b/starters/dev-server.ts @@ -148,19 +148,7 @@ export { `; } if (id.endsWith(qwikCityStaticPaths)) { - return `export function isStaticPath(method, url){ - if (method !== 'GET') { - return false; - } - if (url.search !== '') { - return false; - } - const p = url.pathname; - if (p.includes('/build/')) { - return true; - } - return false; - };`; + return `export function isStaticPath(method, url){ return false; };`; } if (id.endsWith(qwikCityNotFoundPaths)) { const notFoundHtml = getErrorHtml(404, "Resource Not Found"); diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index 4ae9a035ff4..015ee571cac 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -2,8 +2,9 @@ import { expect, test } from "@playwright/test"; test.describe("Qwik City Adapter", () => { test("Qwik City Adapter", async ({ page: api }) => { + const nestedUrl = "by/pass/other-routes-for-testing"; const rsp = (await api.goto( - "/qwikcity-test/adapter/build/a-random-file-after-that.js", + `/qwikcity-test/build/${nestedUrl}/a-random-file-after-that.js`, ))!; expect(rsp.status()).toBe(404); expect(rsp.headers()["content-type"]).toBe("text/html; charset=utf-8"); From 329f490bbf0e7b2f85c13ac28ac69695770d662b Mon Sep 17 00:00:00 2001 From: PatrickJS Date: Mon, 20 May 2024 00:00:36 -0700 Subject: [PATCH 9/9] fix: test name --- starters/e2e/qwikcity/adapter.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/starters/e2e/qwikcity/adapter.spec.ts b/starters/e2e/qwikcity/adapter.spec.ts index 015ee571cac..bac5c960dfb 100644 --- a/starters/e2e/qwikcity/adapter.spec.ts +++ b/starters/e2e/qwikcity/adapter.spec.ts @@ -1,7 +1,9 @@ import { expect, test } from "@playwright/test"; test.describe("Qwik City Adapter", () => { - test("Qwik City Adapter", async ({ page: api }) => { + test("should have simple Not Found response for middleware/node static files", async ({ + page: api, + }) => { const nestedUrl = "by/pass/other-routes-for-testing"; const rsp = (await api.goto( `/qwikcity-test/build/${nestedUrl}/a-random-file-after-that.js`,