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.

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

Update to latest Kit and handle breaking changes #270

Merged
merged 9 commits into from
Jan 21, 2022

Conversation

geoffrich
Copy link
Member

@geoffrich geoffrich commented Jan 13, 2022

This PR updates the repo to the latest version of SvelteKit, and handles the breaking changes around page, query, and the updated Request and Response. This is a prerequisite for #245, which requires the change from next-215 to make the skip link work properly.

@vercel
Copy link

vercel bot commented Jan 13, 2022

@geoffrich is attempting to deploy a commit to the Svelte Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@Conduitry Conduitry left a comment

Choose a reason for hiding this comment

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

We should probably keep using the npm tag next in all of the package.json files for Kit and its adapters.

It also doesn't look like what you're updating to is the latest version anymore.

@geoffrich
Copy link
Member Author

Thanks, I made most of these changes yesterday and missed the new releases. Updated to actual latest and put next back in the package.json

@vercel
Copy link

vercel bot commented Jan 13, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

hn – ./sites/hn.svelte.dev

🔍 Inspect: https://vercel.com/svelte/hn/CTRgBB8csotzYHPYo79Z2vvDBKJS
✅ Preview: https://hn-git-fork-geoffrich-update-kit-svelte.vercel.app

svelte-dev – ./sites/svelte.dev

🔍 Inspect: https://vercel.com/svelte/svelte-dev/HaDUvp5gh7EsKWhYNKDekcGefWcs
✅ Preview: https://svelte-dev-git-fork-geoffrich-update-kit-svelte.vercel.app

@geoffrich
Copy link
Member Author

I'll do some validation on the preview deployments to make sure everything is working properly.

@geoffrich
Copy link
Member Author

Looks like this broke anything involving a Vercel serverless function (server rendering and calling endpoints), though everything works fine locally. Going to mark as draft until I can figure out what's going on.

image

@geoffrich geoffrich marked this pull request as draft January 13, 2022 19:43
@geoffrich
Copy link
Member Author

Blocked by sveltejs/kit#3343

@benmccann
Copy link
Member

benmccann commented Jan 20, 2022

Thanks for fixing sveltejs/kit#3400 @geoffrich! Hopefully you're unblocked now, but let us know if there are any other issues remaining

@geoffrich
Copy link
Member Author

Waiting to leave draft state until I can E2E test, since this touches all functionality on the site (saving/deleting REPLs, login, etc). @benmccann can you authorize the preview deployment of svelte.dev to Vercel?

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

actually I take back my approval 😄 since you've updated the site-kit code I think you'll also need to upgrade kit.svelte.dev to the latest version of SvelteKit as well or else I'm guessing that site will break since it depends on the workspace version of site-kit:

"@sveltejs/site-kit": "workspace:*",

@ignatiusmb ignatiusmb linked an issue Jan 21, 2022 that may be closed by this pull request
@geoffrich
Copy link
Member Author

@benmccann I think I did -- see the pnpm lockfile. It also seems to work properly when running locally.

Finished testing and everything looks good, so I think this is ready for final approval now. Login stuff didn't work on the preview deployment, but I assume that's because the GitHub OAuth is only set up for svelte.dev. Running locally I was able to login, logout, and save & delete REPLs.

@geoffrich geoffrich marked this pull request as ready for review January 21, 2022 16:45
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

ok, yeah, you're right. I expected we would have at least one endpoint needing to be migrated in either the HN or SvelteKit sites, but I couldn't find any. I guess I forgot they're all in api.svelte.dev, so there's not really anything to do here

@benmccann benmccann merged commit 95d4753 into sveltejs:master Jan 21, 2022
@geoffrich geoffrich deleted the update-kit branch January 21, 2022 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kit docs headline overlap
3 participants