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

base configuration option can break vite's raw fs handler in devserver #7520

Open
1 task done
sciolist opened this issue Jun 29, 2023 · 4 comments
Open
1 task done
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs

Comments

@sciolist
Copy link

What version of astro are you using?

2.7.1

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Linux

What browser are you using?

Chrome

Describe the Bug

If you set the base config option to a part of a folder name that exists in the root, vite will not be able to load any files under that path.

This code right here:
https://github.com/withastro/astro/blob/main/packages/astro/src/vite-plugin-astro-server/plugin.ts#L46-L52

Places the 'base' middleware before the raw fs handler, and in the base middleware there's this bit of code:
https://github.com/withastro/astro/blob/main/packages/astro/src/vite-plugin-astro-server/base.ts#L25-L28

If you create an astro.config with the base option set to say, 'src' (like the linked example) where all your files are located, this will cause the urls to any files loaded on the client to get converted from /src/my-file.ts to /my-file.ts, which will result in a 404 as the file is not at that path.

This also happens if you set trailingSlashes to never, in that case it breaks any file with the same prefix as your base path (so in my case, i had a base path of '/no', which turns all '/node_modules' to '/de_modules')

My workaround for now is to move the mw around in the devserver with a vite plugin, so that the base middleware is placed just before astroDevHandler, and moving the viteServePublicMiddleware to run after the base middleware as those files should have their base path stripped.

I'm not sure what the cleanest real solution would be, there are some quick fixes like not modifying the URL if there's an "astro" querystring parameter, but '@vite/client' etc scripts does not include that, so there could still be issues.

What's the expected result?

The raw fs handler in vite should be able to serve any local files in the project regardless of the base configuration option value.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-hajh6e

Participation

  • I am willing to submit a pull request for this issue.
@bluwy
Copy link
Member

bluwy commented Dec 26, 2023

I took a stab fixing this today at https://github.com/withastro/astro/tree/improve-base-handling. However that exploded in scope as I realized Astro doesn't pass the base to Vite, and it changes the base handling order (as you mentioned). Both can possibly break Vite plugin compatibility as Vite plugins can't reliably detect the base.

I think overall it will be a breaking change, so something we could fix later. We can probably scope down the issue first by properly preprending the base to the URLs, e.g. /src/my-file.ts should be /my-base/src/file.ts, which is what Vite does too. I'll try to tackle this later.

@bluwy bluwy self-assigned this Dec 26, 2023
@kmesic
Copy link

kmesic commented Mar 16, 2024

Any timeline to fix this? My use case is on local dev, we have a proxy that routes to the astro app or Vite app. The problem is its hard to determine what Vite files should go where.

Ideally if you add a base in the Astro config, all Vite HMR files are also prefixed with the base. However, that is not the case. I tried adding the same base to the Vite config within Astro - didn't work.

Basically what I want is all files even things like @vite/client to be served from a base url prefix.

@bluwy bluwy added the requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs label May 9, 2024
@patheticGeek
Copy link

@kmesic were you able to work around this?
we also have astro behind a proxy and need to re-route all requests

also, i am not sure if this comes under a minor bug, i think this should be more than that based on it totally breaking the concept of a base url

@ascorbic
Copy link
Contributor

ascorbic commented Dec 2, 2024

In Astro 5/Vite 6, this breaks the page entirely: https://stackblitz.com/edit/github-hajh6e-3yjzwj?file=astro.config.mjs

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) requires refactor Bug, may take longer as fixing either requires refactors, breaking changes, or considering tradeoffs
Projects
None yet
Development

No branches or pull requests

6 participants