Allowing Vite to handle base config for deploying to subpaths #3178

tony-sull · 2022-04-22T11:07:41Z

Changes

Follow-up to #3156, this gets Astro out of the subpath game completely and lets Vite take care of it

The fix here ended up being pretty straight forward, though not totally ideal since we're still handling the base config option ourself in dev

astro dev

The main challenge in development is that we really need Astro.request.url to include the base path, but if Vite is handling the base config it will remove that from the request. i.e. requesting /subpath/images/penguin.png in dev will gives us a URL of /images/penguin.png

This would cause an issue especially when comparing the URL for scenarios like highlighting the current navigation menu item.

astro build

In production builds, this now lets Vite handle the base config. This is necessary to make sure the base path is included for imported assets. i.e. import Penguin from "../images/penguin.png" needs to resolve as /subpath/assets/asset.123.png

For scripts and styles that Astro injects, this change updates the build to include the subpath there as well

Testing

Existing tests mostly cover this already, the static-build suite actually had a bug in one test that was letting it pass today even though the subpath wasn't being included in the build

Docs

None, bug fix only

changeset-bot · 2022-04-22T11:07:43Z

🦋 Changeset detected

Latest commit: 6824dd3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
astro	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

matthewp · 2022-04-22T14:02:54Z

Can you explain the Astro.request.url change more? I assume you mean in dev? Are you saying that:

Current main, with subpath

http:https://localhost:3000/subpath/some/page/

It is now this?

This branch

http:https://localhost:3000/some/page/

tony-sull · 2022-04-22T17:55:16Z

Can you explain the Astro.request.url change more? I assume you mean in dev? Are you saying that:

Yep that's exactly right. Today in main, Astro.request.url will include the subpath similar to http:https://localhost:3000/subpath/some/page/ and with this PR the subpath would no longer be included.

There might be a way we could shim the subpath back into the request URL, I'm open to that if it's needed but didn't find a clean fix yet and it might actually make more sense for the basepath to not be included in request.url (that would match directory-based routing structure)

Phynecs · 2022-04-29T08:32:23Z

I guess this (using vite for handling base directly) would also make it possible to have all assets and links relative like in this example:

In vite I can set the base to empty or './' to make all the links and imports/png/js/css relative (https://vitejs.dev/config/#base)

this works when using vite directly, here on the example of a vite svelte package (the vite.config.js):

export default defineConfig({
  plugins: [svelte()],
  base: './'
})

and wenn building the site with: npm run build
I get relative paths in the index.html in dist:

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <link rel="icon" href="./favicon.ico" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <title>Svelte + Vite App</title>
    <script type="module" crossorigin src="./assets/index.8a09354a.js"></script>
    <link rel="stylesheet" href="./assets/index.cf80e917.css">
  </head>
  <body>
    <div id="app"></div>
    
  </body>
</html>

compared to the current astro.build version where it doesn't work, if I set it to './' or just empty string in astro.config.mjs:

export default defineConfig({
  integrations: [
    tailwind()
  ],
  base: './',
});

I get absoulte paths in the output:

<link rel="stylesheet" href="/assets/asset.a7084969.css"></link>

source Discord support-threads

tony-sull · 2022-05-03T20:57:49Z

packages/astro/src/core/build/generate.ts

- astroConfig.base !== '/'
- ? joinPaths(astroConfig.site?.toString() || 'http:https://localhost/', astroConfig.base)
- : astroConfig.site;
+ const site = astroConfig.base !== '/'


This makes sure that the subpath is always used for scripts and styles that Astro injects into the page

tony-sull · 2022-05-03T20:59:27Z

packages/astro/src/core/build/static-build.ts

@@ -162,7 +162,7 @@ async function ssrBuild(opts: StaticBuildOptions, internals: BuildInternals, inp
 root: viteConfig.root,
 envPrefix: 'PUBLIC_',
 server: viteConfig.server,
- base: astroConfig.site ? new URL(astroConfig.site).pathname : '/',
+ base: astroConfig.base,


This gives Vite the base URL (or the "/" default), making sure that any imported assets include the subpath

i.e. import image from "../images/penguin.png" will resolve to /subpath/assets/asset.123.png

packages/astro/src/core/create-vite.ts

packages/astro/src/vite-plugin-astro-server/index.ts

…)" This reverts commit 637919c.

…tyles

…t imports

tony-sull · 2022-05-04T15:19:35Z

packages/astro/test/static-build.test.js

@@ -96,6 +96,9 @@ describe('Static build', () => {
 for (const link of links) {
 const href = $(link).attr('href');

+ // The imported .scss file should include the base subpath in the href
+ expect(href.startsWith('/subpath/')).to.be.true;


This test had a coverage bug in it that ignored the fact that "/subpath/" wasn't being used at all before this PR

This makes sure the test fixture's ESM import for a .scss file gets prepended with the subpath by Vite's build

…tro#3178) * Revert "Improvements to build and dev when building for subpaths (withastro#3156)" This reverts commit 637919c. * letting Vite handle base paths * test updates to expect Astro.request.url to no longer include subpaths * bringing back the fix for including subpaths in injects scripts and styles * fixing the static-build test to handle subpaths for injected CSS * fixing asset import URLs when using base subpaths * chore: fixing typo in the comments * Astro needs to manage base in dev to maintain Astro.request.url * fix: reverting dev routing tests to expect existing behavior * reverting Astro global test to verify existing behavior * chore: adding changeset * test: update static-build tests to verify the subpath is used in asset imports

github-actions bot added pkg: astro Related to the core `astro` package (scope) test labels Apr 22, 2022

tony-sull self-assigned this Apr 22, 2022

Auroratide mentioned this pull request Apr 28, 2022

🐛 BUG: public folder doesn't get copied to subpath if buildOptions.site is provided in development #2409

Closed

tony-sull force-pushed the fix/vite-config-base branch 2 times, most recently from 18cf3fb to be3723e Compare May 3, 2022 16:32

tony-sull commented May 3, 2022

View reviewed changes

packages/astro/src/core/create-vite.ts Outdated Show resolved Hide resolved

tony-sull commented May 3, 2022

View reviewed changes

packages/astro/src/vite-plugin-astro-server/index.ts Outdated Show resolved Hide resolved

tony-sull force-pushed the fix/vite-config-base branch from a363133 to 3de7685 Compare May 3, 2022 21:12

Tony Sullivan added 7 commits May 4, 2022 09:29

Revert "Improvements to build and dev when building for subpaths (#3156…

6eedfcc

…)" This reverts commit 637919c.

letting Vite handle base paths

85185ff

test updates to expect Astro.request.url to no longer include subpaths

2362937

bringing back the fix for including subpaths in injects scripts and s…

ae1d5c1

…tyles

fixing the static-build test to handle subpaths for injected CSS

424b555

fixing asset import URLs when using base subpaths

875a43e

chore: fixing typo in the comments

3559225

tony-sull force-pushed the fix/vite-config-base branch from 3de7685 to 3559225 Compare May 4, 2022 14:29

Tony Sullivan added 3 commits May 4, 2022 09:51

Astro needs to manage base in dev to maintain Astro.request.url

e894a22

fix: reverting dev routing tests to expect existing behavior

205c3be

reverting Astro global test to verify existing behavior

b39d03b

github-actions bot removed the test label May 4, 2022

Tony Sullivan added 2 commits May 4, 2022 10:01

chore: adding changeset

4c9d7ce

test: update static-build tests to verify the subpath is used in asse…

6824dd3

…t imports

github-actions bot added the test label May 4, 2022

tony-sull commented May 4, 2022

View reviewed changes

tony-sull marked this pull request as ready for review May 4, 2022 15:19

tony-sull changed the title ~~WIP: Allowing Vite to handle base config for deploying to subpaths~~ Allowing Vite to handle base config for deploying to subpaths May 5, 2022

matthewp approved these changes May 5, 2022

View reviewed changes

tony-sull merged commit 19e1686 into main May 5, 2022

tony-sull deleted the fix/vite-config-base branch May 5, 2022 22:39

github-actions bot mentioned this pull request May 5, 2022

[ci] release #3308

Merged

Phynecs mentioned this pull request May 13, 2022

Allow relative base (linking of assets and base) / vite embeded deployment #3360

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allowing Vite to handle base config for deploying to subpaths #3178

Allowing Vite to handle base config for deploying to subpaths #3178

tony-sull commented Apr 22, 2022 •

edited

Loading

changeset-bot bot commented Apr 22, 2022 •

edited

Loading

matthewp commented Apr 22, 2022

tony-sull commented Apr 22, 2022

Phynecs commented Apr 29, 2022

tony-sull May 3, 2022

tony-sull May 3, 2022

tony-sull May 4, 2022

Allowing Vite to handle base config for deploying to subpaths #3178

Allowing Vite to handle base config for deploying to subpaths #3178

Conversation

tony-sull commented Apr 22, 2022 • edited Loading

Changes

Testing

Docs

changeset-bot bot commented Apr 22, 2022 • edited Loading

🦋 Changeset detected

matthewp commented Apr 22, 2022

tony-sull commented Apr 22, 2022

Phynecs commented Apr 29, 2022

tony-sull May 3, 2022

Choose a reason for hiding this comment

tony-sull May 3, 2022

Choose a reason for hiding this comment

tony-sull May 4, 2022

Choose a reason for hiding this comment

tony-sull commented Apr 22, 2022 •

edited

Loading

changeset-bot bot commented Apr 22, 2022 •

edited

Loading