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

Fix subpath script import from subpath html #114

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

elturpin
Copy link
Contributor

I recently discovered that if an html file located in a sub-path of the root imported a script in the sub-path using relative path, vite server would complain that the script was not found because he was searching at the root.

i.e. if we got /subpath/index.html while the a script was at /subpath/script.js

<script type="module" src="./script.js"></script>

vite would search /script.js.

I add test case to give a complete example in the basic env.

I manage to understand that the problem comes from how the vite middleware is done and how the transformIndexHtml of vite is used. From the code of vite (https://github.com/vitejs/vite/blob/6c4bf266a0bcae8512f6daf99dff57a73ae7bcf6/packages/vite/src/node/server/middlewares/indexHtml.ts#L435) I discovered that we can give more context on what is the real path of the template so that the path given inside the template take the right reference.

For the tests, I am not really satisfied of my solution. The problem is that the server manage to serve what it must serve, their is just an error log from vite that tells their is something wrong. So I spy on the console.error calls. I do not now if their is something better to do.

@szymmis
Copy link
Owner

szymmis commented Jan 25, 2024

Hi @elturpin, thanks a lot for catching that bug and fixing it right away!

I've tried to think of a better way to assert that but the only thing that comes to my mind is to use puppeteer which would basically mean moving this assertion to templates tests. I don't want to do because it's a small issue so we'll stick to your solution. The only thing I've done is that I've added a note about that assertion to explain why we even spying on console error.

Thanks again for taking care of this! Will release it quickly as v0.14.1.

@szymmis szymmis merged commit 161d5a6 into szymmis:master Jan 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants