This repository was archived by the owner on Sep 30, 2024. It is now read-only.
fix(svelte): Properly load/serve version.json#63688
Merged
Merged
Conversation
In certain situations SvelteKit tries to load `version.json` to check whether a new version of the web app is available. This currently doesn't work, accessing this file throws a 404 error. This is due to how SvelteKit generates the path to access the file. It uses the value of `appDir` for this, which is currently set to the default value. We put our assets in `client/web/dist/_sk/*`, which can then be accessed on the server via `/.assets/_sk/*`. To make that work properly we've been modifiying the generated `index.html` file to use the right file path. This however doesn't work for fetching `version.json` because the path to that file is generated at runtime. This commit aims to fix this issue by setting `appDir` to a value that works with the Sourcegraph server so that `version.json` can be properly loaded. Incidentally this also makes our "post processing" easier: Instead of changing the contents of the `index.html` file we instead have to move the app assets to the proper location. With `appDir` set to `.assets/_sk/_app`, SvelteKit will put the app assets into `<out>/.assets/_sk/_app`. But that would mean the full URL path to a file would be `.assets/_sk/.assets/_sk/_app`. To fix that we now move the files from `<out>/.assets/_sk/_app` to `<out>/_app`.
camdencheek
approved these changes
Jul 8, 2024
camdencheek
left a comment
Member
There was a problem hiding this comment.
Makes sense to me! Thanks for the explanatory comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes srch-700
In certain situations SvelteKit tries to load
version.jsonto check whether a new version of the web app is available (see the SvelteKit docs for more information). This currently doesn't work, accessing this file throws a 404 error.This is due to how SvelteKit generates the path to access the file. It uses the value of
appDirfor this, which is currently set to the default value. We put our assets inclient/web/dist/_sk/*, which can then be accessed on the server via/.assets/_sk/*. To make that work properly we've been modifiying the generatedindex.htmlfile to use the right file path.This however doesn't work for fetching
version.jsonbecause the path to that file is generated at runtime.This commit aims to fix this issue by setting
appDirto a value that works with the Sourcegraph server so thatversion.jsoncan be properly loaded. Incidentally this also makes our "post processing" easier: Instead of changing the contents of theindex.htmlfile we instead have to move the app assets to the proper location. WithappDirset to.assets/_sk/_app, SvelteKit will put the app assets into<out>/.assets/_sk/_app. But that would mean the full URL path to a file would be.assets/_sk/.assets/_sk/_app. To fix that we now move the files from<out>/.assets/_sk/_appto<out>/_app.Test plan
Tested that
pnpm dev,pnpm previewandbazel test //client/web-sveltekit:e2e_testwork as expected.