-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
chore: Resolve some random TODOs #13328
base: main
Are you sure you want to change the base?
Conversation
|
preview: https://svelte-dev-git-preview-kit-13328-svelte.vercel.app/ this is an automated message |
@@ -317,7 +317,7 @@ async function prerender({ hash, out, manifest_path, metadata, verbose, env }) { | |||
const { pathname, search, hash } = new URL(href, 'http://localhost'); | |||
|
|||
if (search) { | |||
// TODO warn that query strings have no effect on statically-exported pages | |||
log.warn(`Query parameters in ${href} will have no affect on prerendering`); |
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.
Do you think it's worth adding a changeset for this?
@@ -146,8 +146,7 @@ export async function setResponse(res, response) { | |||
key, | |||
key === 'set-cookie' | |||
? set_cookie_parser.splitCookiesString( | |||
// This is absurd but necessary, TODO: investigate why | |||
/** @type {string}*/ (response.headers.get(key)) | |||
value |
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.
Mhhhm @gtm-nayan do you remember why exactly this was necessary? You added it in #9638 - want to make sure that not just using the value really was on purpose
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.
Will have to test again to be exact but IIRC, there can be multiple set-cookie headers and how the value is returned with multiple of them, didn't work with how we use it here.
I was reacquainting myself with the codebase and stumbled upon some random trash
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm test
and lint the project withpnpm lint
andpnpm check
Changesets
pnpm changeset
and following the prompts. Changesets that add features should beminor
and those that fix bugs should bepatch
. Please prefix changeset messages withfeat:
,fix:
, orchore:
.Edits