-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Site: Migrate to Polka #2478
Site: Migrate to Polka #2478
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2478 +/- ##
=========================================
Coverage ? 91.83%
=========================================
Files ? 1
Lines ? 49
Branches ? 0
=========================================
Hits ? 45
Misses ? 4
Partials ? 0 Continue to review full report at Codecov.
|
You can create some local credentials for yourself for testing by creating a GitHub OAuth app - https://github.com/settings/applications/new - https://github.com/sveltejs/svelte/tree/master/site#repl-github-integration |
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.
am testing the auth stuff locally now
send(res, 200, json); |
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.
isn't there a penalty to stringifying per-response rather than doing it once?
}); | ||
if (!example || process.env.NODE_ENV !== 'production') { | ||
example = get_example(slug); | ||
cache.set(slug, example); |
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.
same comment re stringifying JSON
Not sure I understand why, but |
Aye, I forgot I had stashed changes at home for a Technically true about the cached JSON serialization, but removed for declaration consistency. Figured it doesn't really matter because of the 10m+ caching now, plus the internal cache would only be hit once per 10m per region, assuming the GCR instance was still running through the whole hour |
Got the Auth routes running locally. I bummed the values off svelte.dev instead of setting up a separate oAuth app, so aside from the redirect mismatch, all seemed to work out well 👍 I can revert the |
feels good, man |
Relies on #2476 in order to be functional because
serve-static
wasn't actually declared as a dependency, so removingexpress
removed it intrinsically.I've only not tested auth/login (passport + express-session compat) because I don't have the tokens locally to test it.
Added
@polka/send
(next version) to handle responses uniformly. Not sure if the all the varying formats were done over time or by different people 😄 In any event,polka/send
will autocast to JSON and will autosetContent-Type
andContent-Length
. It also does a few other things that we're not using here.Marking as "Draft" until the session/passport stuff can be checked out.