-
Notifications
You must be signed in to change notification settings - Fork 40
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
Mount HTML serving middlewares at config.base #91
Merged
+13
−11
Merged
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
28fd536
add ignore base option
bdcbe68
Vite Index Middleware should only handle GET requests
f529b28
Use base directly instead of introducing another config option
5079964
Specified base must be included in the page address
a369482
Add optional base parameter in the testCase
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost got there 😅, what I suggest is to add
base
parameter totestCase
function, that's set to/
by default.That's because right now other test would fail (Well, maybe they wouldn't fail because if vite-express is mounted at root, it will serve the page at
/admin
, but it will definitely make things unclear). If you want to run them locally you can usenpm run test
oryarn test
or any other package manager.By specifying the parameter as default, you don't need to adjust other
testCase
invocations, only the one inTemplate "${template}" with custom inline config
test here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made modifications to the code, but on windows, even after attempting to install with both npm and yarn, I consistently encounter the following error when running
npm run test
with or without administrative privileges:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's interesting, I'm running linux so that's why I didn't catch it, but thanks for bringing that up. You can push the code so I can take a look and run the CI if everything looks alright.