Express refactor: Static file routes and templates #2246

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

Jump to bottom

Closed

GirlBossRush wants to merge 1 commit into master from handlebars-templates

Closed

Express refactor: Static file routes and templates #2246

GirlBossRush wants to merge 1 commit into master from handlebars-templates

Conversation

GirlBossRush

Copy link

Contributor

@GirlBossRush GirlBossRush commented Oct 28, 2020 •

edited

Loading

This PR continues the Expressification of code-server.

Progress

Refactors asset path helpers and removes csStaticPath concept
Fixed issue where JSON entities may be unescaped
Updated web manifest schema, route handler
Fixed issue where body background color is often out of sync on startup and after theme change
Added types to vscode route.
Adds /local route for authenticated static file serving

Closes #1292

@GirlBossRush GirlBossRush added the enhancement label

Oct 28, 2020

@GirlBossRush GirlBossRush requested a review from code-asher

October 28, 2020 21:18

@GirlBossRush GirlBossRush requested a review from nhooyr as a code owner

October 28, 2020 21:18

@GirlBossRush GirlBossRush self-assigned this

Oct 28, 2020

code-asher

code-asher requested changes

Nov 3, 2020

View reviewed changes

Copy link

Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Handlebars + static combo-wombo! This looks great! ty again for taking care of this.

ci/dev/watch.ts Outdated Show resolved Hide resolved

src/node/routes/index.ts Outdated Show resolved Hide resolved

src/node/routes/login.ts Show resolved Hide resolved

src/node/http.ts Outdated Show resolved Hide resolved

src/node/routes/manifest.ts Outdated Show resolved Hide resolved

src/node/http.ts Outdated Show resolved Hide resolved

src/node/routes/index.ts Outdated Show resolved Hide resolved

src/node/routes/manifest.ts Show resolved Hide resolved

src/browser/views/error.handlebars Outdated Show resolved Hide resolved

src/node/app.ts Show resolved Hide resolved

@GirlBossRush GirlBossRush force-pushed the handlebars-templates branch from faed813 to 4140b09 Compare

November 3, 2020 22:50

@GirlBossRush GirlBossRush requested a review from code-asher

November 4, 2020 14:56

@GirlBossRush GirlBossRush force-pushed the handlebars-templates branch 3 times, most recently from 6fece46 to d8fe245 Compare

November 4, 2020 18:25

code-asher

code-asher reviewed

Nov 4, 2020

View reviewed changes

Copy link

Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Latest changes look great! There are still some unresolved comments (looks like they're being collapsed and hidden though).

src/browser/views/vscode/index.ts

}

// Then, observe the meta theme element for changes.

const themeElement = document.getElementById("monaco-workbench-meta-theme-color") as HTMLMetaElement

Copy link

Member

@code-asher code-asher Nov 4, 2020 •

edited

Loading

Choose a reason for hiding this comment

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

This is neat! In the future we could even update local storage with this method instead of doing it in the patch. It's always a happy day when we can reduce the patch size.

@code-asher code-asher force-pushed the code-asher/ch1385 branch from ef7ce56 to 3f1750c Compare

November 5, 2020 21:08

@GirlBossRush GirlBossRush force-pushed the handlebars-templates branch from a46e01f to 97bd72d Compare

November 5, 2020 21:42

@GirlBossRush GirlBossRush changed the title ~~(削除) Express refactor: Handlebars templates (削除ここまで)~~ (追記) Express refactor: Static file routes and templates (追記ここまで)

Nov 5, 2020

@GirlBossRush GirlBossRush force-pushed the handlebars-templates branch from e90c633 to 19f5971 Compare

November 6, 2020 19:49

@GirlBossRush


 Refactor template string with handlebars.

34b52a0

- Refactor static routing, template vars.
- Fixed issue where JSON entities may be unescaped
- Updated web manifest schema, route handler.
- Clean up issues surrounding static paths, types.
- Removed static path helper.
- Fixed issue where locals may change during request handling.
- Added missing types.
- Fixed issue where body theme color is stale.
- Refactor missing tar endpoint.
- Add local route handler.

@GirlBossRush GirlBossRush force-pushed the handlebars-templates branch from 19f5971 to 34b52a0 Compare

November 6, 2020 20:04

Base automatically changed from code-asher/ch1385 to master

November 12, 2020 20:04

ghost

ghost approved these changes

Nov 21, 2020

View reviewed changes

@code-asher

Copy link

Member

code-asher commented Nov 30, 2020

I haven't been able to re-review this yet. The divergence is growing and I'm not totally sure the best way to proceed but here's my plan:

Close to keep the PR queue slim.
Rebase on master.
Split up the commit a bit.
Open a new PR.

Additionally, we had a discussion about Handlebars and we want to try taking a route that lets us have type checking in the template (if we do though it'll basically just be replacing the .hbs files, so not a huge departure).

@code-asher code-asher closed this

Nov 30, 2020

@nhooyr nhooyr deleted the handlebars-templates branch

January 8, 2021 00:55

@nhooyr nhooyr restored the handlebars-templates branch

January 8, 2021 00:56

@nhooyr

Copy link

Contributor

nhooyr commented Jan 8, 2021

Deleted this branch for now. I have it locally and saved on my fork in case we need to access these commits again. Just trying to keep the main repositories branches clean.

@nhooyr nhooyr deleted the handlebars-templates branch

January 8, 2021 00:57

@nhooyr

Copy link

Contributor

nhooyr commented Jan 8, 2021

For posterity, the link is https://github.com/nhooyr/code-server/tree/z/handlebars-templates

Labels

enhancement

3 participants

@GirlBossRush @code-asher @nhooyr

Express refactor: Static file routes and templates #2246

Express refactor: Static file routes and templates #2246

Uh oh!

Conversation

@GirlBossRush GirlBossRush commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Uh oh!

@code-asher code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

@code-asher code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

@code-asher code-asher Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher commented Nov 30, 2020

Uh oh!

nhooyr commented Jan 8, 2021

Uh oh!

nhooyr commented Jan 8, 2021

Uh oh!

Uh oh!

@GirlBossRush GirlBossRush commented Oct 28, 2020 •

edited

Loading

@code-asher code-asher Nov 4, 2020 •

edited

Loading