You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
* chore: remove unused deno.json field
When running deno, it warns that this is unused when also specifying `imports`.
* chore: fix type errors in Deno files
We weren't including Deno files in `tsc` typechecking, but we also weren't using any `deno` commands
that typecheck. This fixes obscured errors.
I wanted to introduce typechecking in CI in this commit, but it turned out harder than I expected,
so I've pulled that into a separate PR.
The JSON hack isn't great but really it's the whole "import a json and write new data to it" pattern
that should be simplified to "just read the new data in memory", since there's no actual reason to
write it to disk if you follow the code paths. It was just done that way because it was the shortest
path to add the dynamic fissue annotation from the previous implementation.
* chore(e2e-report): fix handling of skipped suites
Some parts of the code weren't correctly handling the case where an entire suite is skipped, with no
specific skipped tests specified.
1. The `testCount.skipped` before this fix was strangely the number of specifically skipped tests
plus the total number of suites which are either entirely skipped or contain skipped tests (i.e.
it was pretty wrong).
2. The computed `skipped` count within each test suite was also wrong but in a different way. It was
not counting at all skipped tests where the entire suite is skipped.
I've also commented some existing correct logic that is very unintuitive.
* chore(e2e): split open issues and skipped tests
It was really confusing, both as a person viewing the page, and as a developer working with the
code. This code and this component were muddling three different data types: failed tests, skipped
tests, and skipped test suites. It was really error prone and hard to follow and modify.
I've split this into separate components and sections on the page for failed tests ("open issues")
and skipped tests, and I've passed skipped tests and skipped suites separately into the latter
component.
I've also refactored some code with side effects to be easier to work with.
* refactor(e2e): remove stale todo
* chore(e2e): update e2e test-results.json fixture
This is the latest run on "latest" against main, from last night.
* chore: remove unused deno scripts
* chore(e2e): add test-results.json fixture to second path
We expect this in two different locations depending on the code path... Having a sample file here
allows for TypeScript to infer the type correctly.
* chore(e2e): add back some skipped tests we removed
The tests were named on canary, but we still need the old test names for `latest`.
* chore(e2e): fix bad skipped test name, skip 2 more
* chore(e2e): add missing tests to e2e-skip-retry.json
* chore(e2e): add one more skipped test
* chore(e2e): fix appending of skipped suites
It was appending entirely skipped suites AND partially skipped suites.
* chore(e2e): update test-results.json fixtures again
* chore(e2e): add sample issues.json for local dev
"body": "I'm not quite sure exactly what the scope of this is, but the failing tests here have a fixture site that enables `skipMiddlewareUrlNormalize` and a user middleware that redirects any other casing of `/en/*` to `/en/*` (and so on), but when `/EN` is fetched it redirects to `/en/en` instead of `/en`:\r\n\r\n```\r\n ● skip-trailing-slash-redirect › should be able to redirect locale casing $1\r\n\r\n expect(received).toBe(expected) // Object.is equality\r\n\r\n Expected: \"/en\"\r\n Received: \"/en/en\"\r\n\r\n 155 | const res = await next.fetch(`/${locale}`, { redirect: 'manual' })\r\n 156 | expect(res.status).toBe(307)\r\n > 157 | expect(new URL(res.headers.get('location'), 'http://n').pathname).toBe(\r\n | ^\r\n 158 | `/${locale.toLowerCase()}`\r\n 159 | )\r\n 160 | }\r\n\r\n at toBe (e2e/skip-trailing-slash-redirect/index.test.ts:157:73)\r\n\r\n ● skip-trailing-slash-redirect › should be able to redirect locale casing $1\r\n\r\n expect(received).toBe(expected) // Object.is equality\r\n\r\n Expected: \"/ja-jp\"\r\n Received: \"/ja-jp/ja-jp\"\r\n\r\n 155 | const res = await next.fetch(`/${locale}`, { redirect: 'manual' })\r\n 156 | expect(res.status).toBe(307)\r\n > 157 | expect(new URL(res.headers.get('location'), 'http://n').pathname).toBe(\r\n | ^\r\n 158 | `/${locale.toLowerCase()}`\r\n 159 | )\r\n 160 | }\r\n\r\n at toBe (e2e/skip-trailing-slash-redirect/index.test.ts:157:73)\r\n```\r\n\r\nIt looks like we [previously claimed to have fixed this in FRA-332](https://github.com/netlify/next-runtime-minimal/pull/287), but it must have regressed at some point.\r\n\r\ntest: test/e2e/skip-trailing-slash-redirect/index.test.ts\r\nreason: does not correctly handle user middleware that redirects to path with canonical locale casing when app enables `skipMiddlewareUrlNormalize` and path contains locale slug with non-canonical casing",
4
+
"number": 564
5
+
},
6
+
{
7
+
"body": "A pages router site with both basepath and i18n enabled does not match middleware targeted at the root unless a locale is in the URL. It should match when using the default locale\r\n\r\nDemo: \r\n- https://6634b69452829cd391cab206--next-e2e-tests.netlify.app/root/ does not have `x-from-middleware` header\r\n- https://6634b69452829cd391cab206--next-e2e-tests.netlify.app/root/en/ does have `x-from-middleware` header\r\n\r\ntest: test/e2e/middleware-matcher/index.test.ts\r\nreason: Middleware does not match when using basePath and default locale",
8
+
"number": 454
9
+
},
10
+
{
11
+
"body": "There are a few scenarios where our the custom matchers from middleware are not correctly being transformed when using i18n and default locales, as they're either matching too broadly or too narrowly. See the two linked GH issues for details:\r\n\r\n[https://github.com/netlify/next-runtime-minimal/issues/453](https://github.com/netlify/next-runtime-minimal/issues/453)<br>[https://github.com/netlify/next-runtime-minimal/issues/454](https://github.com/netlify/next-runtime-minimal/issues/454)\r\n\r\ntest: Middleware custom matchers i18n should not match\r\nreason: Middleware matching is too broad when using i18n",
12
+
"number": 453
13
+
},
14
+
{
15
+
"body": "On sites that use pages router and have middleware, loading a page using next/link will attempt to load a JSON file, which will return a 404. If there is no middleware then it works fine. This applies even if the middleware does nothing.\r\n\r\ntest: test/e2e/middleware-base-path/test/index.test.ts\r\nreason: Pages router data requests returning 404 when middleware is used\r\ntest case: \"Middleware base tests router.query must exist when Link clicked page routing\"",
16
+
"number": 450
17
+
},
18
+
{
19
+
"body": "When appending a `set-cookie` header, the server returns two copies of the header. e.g. \r\n\r\n```ts\r\nexport async function middleware(request, ev) {\r\n const next = NextResponse.next()\r\n next.headers.append('set-cookie', 'bar=chocochip')\r\n return next\r\n}\r\n```\r\n\r\nLeads to: \r\n\r\n\r\n\r\ntest: test/e2e/middleware-responses/test/index.test.ts\r\nreason: Appending set-cookie header in middleware leads to duplicate header\r\n",
20
+
"number": 447
21
+
},
22
+
{
23
+
"body": "The test [\"app-dir action handling fetch actions should store revalidation data in the prefetch cache\"](https://github.com/vercel/next.js/blob/6475431a4cbbf2b71c38158e0e722183779faf4f/test/e2e/app-dir/actions/app-action.test.ts#L980) fails when running tests, but seems to work fine when testing manually. It may be a first-run issue, which needs investigation.\r\n\r\ntest: test/e2e/app-dir/actions/app-action.test.ts\r\nreason: Fetch action prefetch cache test is flakey",
24
+
"number": 444
25
+
},
26
+
{
27
+
"body": "If a Next.js page returns a 500 error and the browser sent accept-encoding gzip, it seems the returned data _is_ encoded, but no encoding header is set, so the browser cannot decode it.\r\n\r\n`curl --request GET --url https://66056f2be8186e00aaea53d3--next-e2e-tests.netlify.app/enoent --header 'Accept-Encoding: gzip'`\r\n\r\ntest case: https://github.com/vercel/next.js/blob/canary/test/e2e/getserversideprops/test/index.test.ts#L367\r\ntest: test/e2e/getserversideprops/test/index.test.ts\r\nreason: Server error pages return encoded data without content-encoding header if accept-encoding is gzip",
28
+
"number": 387
29
+
},
30
+
{
31
+
"body": "When redirecting a data request, middleware returns a response with `x-nextjs-redirect`, rather than a `location` header. We handle this correctly. However Next.js expects us to directly return and empty response with a 302 response code (without the location header), whereas we're currently passing the request on to the origin and returning the body with 404 code. I'm unsure if it's legal to return a 302 with no location, but it's what next start does, and the router expects.\r\n\r\ntest case: https://github.com/vercel/next.js/blob/canary/test/e2e/middleware-redirects/test/index.test.ts#L100\r\ntest: test/e2e/middleware-redirects/test/index.test.ts\r\nreason: Pages router middleware should return 302 status for redirected data requests",
32
+
"number": 386
33
+
},
34
+
{
35
+
"body": "In sites with trailing slashes enabled, requests should not add a trailing slash when they're for non-data static files.\r\n\r\ntest case: https://github.com/vercel/next.js/blob/canary/test/e2e/middleware-trailing-slash/test/index.test.ts#L436-L437\r\ntest: test/e2e/middleware-trailing-slash/test/index.test.ts\r\nreason: Middleware should not add trailing slashes to non-data requests in static dir",
36
+
"number": 385
37
+
},
38
+
{
39
+
"body": "Rewrites in middleware on i18n sites add the locale to the target automatically. However it is supposed to skip this if the target is a static file. This does not currently work, as the middleware doesn't know if a static file exists. This is documented as a known issue.\r\n\r\ntest: test/e2e/i18n-ignore-rewrite-source-locale/rewrites-with-basepath.test.ts, test/e2e/i18n-ignore-rewrite-source-locale/rewrites.test.ts\r\nreason: Middleware on sites with i18n cannot rewrite to static files",
40
+
"number": 383
41
+
},
42
+
{
43
+
"body": "It doesn't seem to be documented, but the e2e test and fixture here expects a CSP to be automatically applied to scripts in the head. The docs show manually setting it, but [the fixture](https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app/middleware.js#L30) seems to only set it in the response. Nevertheless, running `next start` does seem to set it automatically, but when deployed it doesn't. I think this is low priority because it seems to be undocumented behaviour.\r\n\r\ntest case: https://github.com/vercel/next.js/blob/canary/test/e2e/app-dir/app/index.test.ts#L1711\r\ntest: test/e2e/app-dir/app/index.test.ts\r\nreason: Nonce not automatically set in script tags when using CSP",
0 commit comments