Skip to content

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Sep 23, 2021

Previously, our redirect logic did not work when running in dev mode, since that code was only present in the production server. Now that logic is factored out into a Koa middleware, so that it can run in both places.

I also made a few other changes to make working on the server easier going forward.

In an upcoming PR, I'll be adding a middleware to deal with GitHub auth tokens, so now it will be easier to run that both in production and during local development.

@github-actions
Copy link

A live preview of this PR will be available at the URL(s) below.
The latest URL will be appended to this comment on each push.
Each build takes ~5-10 minutes, and will 404 until finished.

https://pr513-5f3a03a---lit-dev-5ftespv5na-uc.a.run.app/

Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

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

LGTM. Also checked it in manually and navigated to a redirect. 👏

/**
* Creates a Koa middleware that performs lit.dev redirection logic.
*/
export const redirectMiddleware = (): Koa.Middleware => async (ctx, next) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice refactor

@@ -1,13 +1,13 @@
{
"compilerOptions": {
"incremental": true,
"tsBuildInfoFile": "./tsconfig.tsbuildinfo",
"tsBuildInfoFile": "./lib/.tsbuildinfo",
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny nit: Other packages kept this as "./lib/tsconfig.tsbuildinfo".

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm interesting. The default is actually <outdir>/.tsbuildinfo which matches this path (https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-4.html). So we could also just omit it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I might go through and standardize these in a followup.

@aomarks aomarks merged commit 5ff4d2d into main Sep 23, 2021
@aomarks aomarks deleted the devserver branch September 23, 2021 17:04
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.

2 participants