Svelte: inject into primary app.html#63088
Conversation
There was a problem hiding this comment.
Instead of trying to reproduce everything that's in the frontend app.html, instead we can build this to be easily parseable and inject it into app.html using Go's templating that we already use heavily.
There was a problem hiding this comment.
Related: this PR is an alternative to my earlier draft in which I manually reconciled the two templates. I think this is a generally better approach since we don't need to try to keep those files in sync.
There was a problem hiding this comment.
Now that injecting the right application logic is fully the responsibility of frontend, we don't need these sets anymore. sg start enterprise works just fine with the feature flags and switching back and forth.
There was a problem hiding this comment.
I'm not sure that's true. The SvelteKit app still has to be built. If you've built them before then they might or might not be already in the system, but they wouldn't be updated if you e.g. make changes (which doesn't work reliably with these commands either tbf).
That's really the main difference with these commands: They additionally start the build process for the SvelteKit web app. How the HTML file is generated is irrelevant.
There was a problem hiding this comment.
(which doesn't work reliably with these commands either tbf).
Ah, gotcha. I was always under the impression that I had to build manually for these commands to work. Okay, I'll roll back these changes
There was a problem hiding this comment.
It might be fine to remove them. I don't think the rebuilding actually works. The process often terminates. In other words it doesn't really work reliably.
There was a problem hiding this comment.
Oh, okay, unreverting then 🙂 I think we should probably build sveltekit by default with sg start enterprise at this point. I'll work on that as a followup
There was a problem hiding this comment.
This seemed to only be used by sg start enterprise-sveltekit, which no longer exists now that we use the same app.html for everything. I think this could still be used to run a dev server with a local instance, but I think it's probably more intuitive to just run pnpm dev with an overriden SOURCEGRAPH_API_URL. Am I off base here?
There was a problem hiding this comment.
See my comment below. I believe to remember that I added this separately because otherwise the request would be handled by the esbuild web server, which will trigger a rebuild of the react app (unnecessarily) which is rather slow (comparatively).
b127eea to
273aff0
Compare
273aff0 to
4d00aba
Compare
| if sveltekit.Enabled(r.Context()) { | ||
| return sveltekit.RenderTemplate(w, common) | ||
| } | ||
|
|
There was a problem hiding this comment.
This makes me very happy. I always thought it would be nice to eventually do the switch in renderTemplate so that it "just works" everywhere.
There was a problem hiding this comment.
I'm not sure that's true. The SvelteKit app still has to be built. If you've built them before then they might or might not be already in the system, but they wouldn't be updated if you e.g. make changes (which doesn't work reliably with these commands either tbf).
That's really the main difference with these commands: They additionally start the build process for the SvelteKit web app. How the HTML file is generated is irrelevant.
There was a problem hiding this comment.
See my comment below. I believe to remember that I added this separately because otherwise the request would be handled by the esbuild web server, which will trigger a rebuild of the react app (unnecessarily) which is rather slow (comparatively).
488d197 to
4d00aba
Compare
|
@fkling were there any followups you were looking for from your comments? |
This modifies our pattern for shipping a svelte-enabled HTML document by injecting the svelte-specific things into our primary
app.htmlrather than trying to makeapp.prod.htmlmatch the behavior of the frontendapp.html.I could not figure out a "blessed" way to integrate this into the build system, so I went for the hacky way: render
app.prod.htmlin a format that's easily parseable, then parse it infrontendand inject it into ourapp.htmlwith the standard go template patterns.Fixes SRCH-76
Test plan
Manual testing with
sg start enterpriseandsg start enterprise-sveltekit. Manual inspection of the generated HTML and diffing against the previously generated HTML.