fix!: return 404 for resources requests outside the base path #5657
Conversation
This file contains 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
patak-dev
changed the title
sapphi-red
added
the
p3-minor-bug
sapphi-red
reviewed
Sep 16, 2023
This was referenced Oct 12, 2023
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
|
qwik and vitest are failing with main branch too. |
sapphi-red
previously approved these changes
Oct 12, 2023
sapphi-red
previously approved these changes
Oct 12, 2023
|
Maybe we could also add a similar hint like SvelteKit? Seems like a useful addition. I also think we should have a test for this before merging. |
|
Added a message same with SvelteKit and also added a test 👍 |
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
bluwy
changed the title
patak-dev
approved these changes
Oct 12, 2023
Closed
4 tasks
mbland
added a commit
to mbland/vitest-utils-browser-import
that referenced
this pull request
Dec 6, 2023
These changes helped me find the definitive source of the problem in Vite 5.0.0-beta.8: - "fix!: return 404 for resources requests outside the base path" vitejs/vite@40fd2d9 vitejs/vite#5657 vitejs/vite#5656 https://vitejs.dev/guide/migration.html#advanced - Related: vuejs/vitepress#3239 sveltejs/kit#2958 Of course, this is effectively a bug fix for Vite, pointing at a bug in Vitest. Vite is now more strict about enforcing the Public Base Path, and Vitest needs to propagate config.base somehow: - https://vitejs.dev/config/shared-options.html#base - https://vitejs.dev/guide/build.html#public-base-path I'll explain the workaround in the next commit. What follows describes how my methodology to pinpoint the problem evolved and concluded. --- Adding the vite/packages/vite as a "file:" dependency and adding its directory as part of the workspace proved challenging. The TypeScript compiler (`tsc`) kept erroring out on the `tsc --emitDeclarationOnly --outDir temp/node -p src/node` step of the build. It took a few tries to realize that I needed to remove vite/node_modules before every "pnpm build" in vite/. Somehow some stale artifacts were poisoning subsequent runs. That left the problem of running `pnpm i` in this project, which would still try to rebuild vite/packages/vite and choke on the TypeScript. After skimming vite/README.md, I realized I could try adding vite/packages/vite as a "link:" dependency instead of a "file:" dependency. I also learned I could use pnpm.overrides in package.json to ensure other packages used this local version. I kept vitest/packages/* in the workspace to make sure they used this linked version of vite/packages/vite. After that, I could now reliably build the two local packages, and have the local vitest use the local vite. Each time in the ../vite repository, I would: ```sh rm -rf packages/vite/node_modules git reset --hard <tag or commit> pnpm i --frozen-lockfile pnpm build ``` Then I'd go back to this repository and: ```sh pnpm i pnpm test ``` And here's the results of my bisection (I didn't actually use `git bisect`, but followed the same essential process): ```text v5.0.0-beta.12 FAIL v5.0.0-beta.6 OK v5.0.0-beta.9 FAIL v5.0.0-beta.7 OK v5.0.0-beta.8 FAIL ```text Once I had my culprit, I went back into ../vite and ran: ```sh gitk v5.0.0-beta.7..v5.0.0-beta.8 ``` And once I did that, I could see the the commit immediately after v5.0.0-beta.7 affected base path handling. So I repeated the process, setting `<tag or commit>` in ../vite to 40fd2d9bf4073420e6c334f48dc3b63558b688ce. Indeed, this build failed, confirming that commit as the culprit.
6 tasks
This was referenced Feb 11, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 #5656
Description
Return 404 for resources outside the base path
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).