feat: switch to buildApp hook instead of invoking Vite build twice - #15532
Conversation
|
| experimental: { | ||
| renderBuiltUrl() { | ||
| // We could always use a relative asset base path here, but it's better for performance not to. | ||
| // E.g. Vite generates `new URL('/asset.png', import.meta).href` for a relative path vs just '/asset.png'. | ||
| // That's larger and takes longer to run and also causes an HTML diff between SSR and client | ||
| // causing us to do a more expensive hydration check. | ||
| return { | ||
| relative: kit.paths.relative !== false || !!kit.paths.assets | ||
| }; | ||
| } | ||
| }, |
There was a problem hiding this comment.
We had to move this to experimental.renderBuiltUrl because we can't configure the base path per environment. Previously, we were setting a different base path for ssr/client and that was possible because the config plugin hook would run twice per the two Vite builds
There was a problem hiding this comment.
Moving this to the client environment config only causes a weird discrepancy between ssr/client. Inlined styles will be loaded again due to a URL mismatch between the two. Therefore, it's better to keep it at the top-level where it affects the server environments too
![vercel[bot]](https://avatars.github.com/in/8329?s=60&v=4){kind=small}
|
Seems like adding EDIT: ok so we need to set it true at the top-level for the first fix to work, then set it to false in the client environment to disable it there. Setting EDIT 2: ok that did not help because it made the client output absolute paths. We need to manually fix things using renderBuiltUrl |
Rich-Harris
left a comment
Rich-Harris
left a comment
There was a problem hiding this comment.
I don't 100% understand what's going on in renderBuiltUrl but the tests are green so I see no reason not to merge!
closes #16530 v2 was unaffected because the secondary client build re-read the Vite config and created fresh plugin instances, so the extra client-side pushes landed in an array nobody read. #15532 made one instance serve both environments. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [x] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.
3 participants
closes #11919
(plugin authors are responsible for declaring which environments they run in)
This PR moves our server/client/service worker build to take advantage of the new
buildApphook https://main.vite.dev/guide/api-environment-frameworks#environments-during-build . Instead of running the Vite build twice, we are setting the environment config settings ahead of time, then building each environment in thebuildApphook.This is the one of the first steps to adopting the environment API.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits