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

assets: embedded asset server #5622

Merged
merged 7 commits into from
Mar 24, 2022
Merged

assets: embedded asset server #5622

merged 7 commits into from
Mar 24, 2022

Conversation

nicksieger
Copy link
Member

@nicksieger nicksieger commented Mar 23, 2022

During release builds, copy js assets to pkg/assets/build for embedding in the Tilt executable using go:embed. When in --web-mode=prod, prefer embedded asset server to prod asset server if embedded assets are available, identified by index.html being present.

Also, PrecompiledWebMode is replaced by this embedded server as well, since both run from the result of make build-js.

@nicksieger nicksieger requested review from nicks, milas and landism March 23, 2022 01:19
@nicksieger nicksieger force-pushed the nicksieger/embedded-assets branch from 8fd0236 to 23286bf Compare March 23, 2022 14:35
Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

yessssssssss!!!!

http.Handler
}

func NewEmbeddedServer() (Server, error) {
Copy link
Member

Choose a reason for hiding this comment

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

i think the more idiomatic API is something like:

GetEmbeddedServer() (Server, bool)

since "the server doesn't exist" isn't something that you're treating as an error. here's an example of an API that does this: https://pkg.go.dev/os#LookupEnv

@nicksieger nicksieger force-pushed the nicksieger/embedded-assets branch from 8c30625 to d0588e1 Compare March 23, 2022 22:33
@nicksieger nicksieger force-pushed the nicksieger/embedded-assets branch from d0588e1 to ef64b94 Compare March 23, 2022 22:34
Copy link
Member

@nicks nicks left a comment

Choose a reason for hiding this comment

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

LGTM!

@nicksieger nicksieger merged commit 3151f41 into master Mar 24, 2022
@nicksieger nicksieger deleted the nicksieger/embedded-assets branch March 24, 2022 14:59
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.

Unable to show Tilt UI while on a disconnected network feature request: load web assets from offline
2 participants