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.

Sign up for GitHub

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

Jump to bottom

build: upgrade edge-runtime #67565

Merged
merged 5 commits into from
Jul 11, 2024
Merged

build: upgrade edge-runtime #67565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1b8b68
build: upgrade edge-runtime
Kikobeats Jul 8, 2024
4186e9a
test: update error message assertion
lubieowoce Jul 8, 2024
d33d850
fix: don't clobber set-cookie headers if multiple are present
lubieowoce Jul 9, 2024
7dbb753
Merge branch 'canary' into edge-runtime
huozhi Jul 11, 2024
cf05835
Merge branch 'canary' into edge-runtime
ztanner Jul 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"@babel/plugin-proposal-object-rest-spread": "7.20.7",
"@babel/preset-flow": "7.22.5",
"@babel/preset-react": "7.22.5",
"@edge-runtime/jest-environment": "2.3.10",
"@edge-runtime/jest-environment": "3.0.0",
"@emotion/cache": "11.11.0",
"@emotion/react": "11.11.1",
"@fullhuman/postcss-purgecss": "1.3.0",
Expand Down
8 changes: 4 additions & 4 deletions packages/next/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,9 @@
"@babel/traverse": "7.22.5",
"@babel/types": "7.22.5",
"@capsizecss/metrics": "3.2.0",
"@edge-runtime/cookies": "4.1.1",
"@edge-runtime/ponyfill": "2.4.2",
"@edge-runtime/primitives": "4.1.0",
"@edge-runtime/cookies": "5.0.0",
"@edge-runtime/ponyfill": "3.0.0",
"@edge-runtime/primitives": "5.0.0",
"@hapi/accept": "5.0.2",
"@jest/transform": "29.5.0",
"@jest/types": "29.5.0",
Expand Down Expand Up @@ -238,7 +238,7 @@
"debug": "4.1.1",
"devalue": "2.0.1",
"domain-browser": "4.19.0",
"edge-runtime": "2.5.4",
"edge-runtime": "3.0.0",
"events": "3.3.0",
"find-up": "4.1.0",
"fresh": "0.5.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/compiled/@edge-runtime/cookies/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"@edge-runtime/cookies","version":"4.1.1","main":"./index.js","license":"MPL-2.0"}
{"name":"@edge-runtime/cookies","version":"5.0.0","main":"./index.js","license":"MIT"}
2 changes: 1 addition & 1 deletion packages/next/src/compiled/@edge-runtime/ponyfill/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"@edge-runtime/ponyfill","version":"2.4.2","main":"./index.js","types":"./index.d.ts","license":"MPL-2.0"}
{"name":"@edge-runtime/ponyfill","version":"3.0.0","main":"./index.js","types":"./index.d.ts","license":"MIT"}
2 changes: 1 addition & 1 deletion packages/next/src/compiled/@edge-runtime/primitives/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"@edge-runtime/primitives","version":"4.1.0","main":"./index.js","license":"MPL-2.0"}
{"name":"@edge-runtime/primitives","version":"5.0.0","main":"./index.js","license":"MIT"}
2 changes: 1 addition & 1 deletion packages/next/src/compiled/edge-runtime/index.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/next/src/compiled/edge-runtime/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"edge-runtime","main":"index.js","license":"MPL-2.0"}
{"name":"edge-runtime","main":"index.js","license":"MIT"}
16 changes: 10 additions & 6 deletions packages/next/src/server/next-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1623,16 +1623,20 @@ export default class NextNodeServer extends BaseServer<
return { finished: true }
}

for (let [key, value] of result.response.headers) {
if (key.toLowerCase() !== 'set-cookie') continue
// Split compound (comma-separated) set-cookie headers
Copy link
Member

@lubieowoce lubieowoce Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a 100% sure what the goal of this section is -- "Split compound (comma-separated) set-cookie headers" is my best guess. this change seems to fix the failing tests, not sure if it can have any downstream consequences.

(the issue was that the new edge-runtime packages seem to emit multiple set-cookie headers sometimes, and this loop was doing headers.delete('set-cookie') which clears all of them, which means that we'd only ever emit the first one. this is why the tests that expected multiple cookies were failing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @wyattjoh mind taking a look? you added this bit, maybe you have some context for why it was done that way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be required, as the set-cookie header when iterated should emit one cookie each. Can we validate this behaviour using a unit test?

if (result.response.headers.has('set-cookie')) {
const cookies = result.response.headers
.getSetCookie()
.flatMap((maybeCompoundCookie) =>
splitCookiesString(maybeCompoundCookie)
)

// Clear existing header.
result.response.headers.delete(key)
// Clear existing header(s)
result.response.headers.delete('set-cookie')

// Append each cookie individually.
const cookies = splitCookiesString(value)
for (const cookie of cookies) {
result.response.headers.append(key, cookie)
result.response.headers.append('set-cookie', cookie)
}

// Add cookies to request meta.
Expand Down
93 changes: 42 additions & 51 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion test/e2e/middleware-general/test/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ describe('Middleware Runtime', () => {
const payload = readMiddlewareJSON(response)
expect('error' in payload).toBe(true)
expect(payload.error.name).toBe('AbortError')
expect(payload.error.message).toContain('The operation was aborted')
expect(payload.error.message).toContain('This operation was aborted')
})

it(`should validate & parse request url from any route`, async () => {
Expand Down
Loading