-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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(optimizer): resolve relative path by Vite resolver (fixes #8704) #8706
fix(optimizer): resolve relative path by Vite resolver (fixes #8704) #8706
Conversation
✅ Deploy Preview for vite-docs-main ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
d853bf1
to
79c10e6
Compare
Looks good to me. @sapphi-red could there be performance implications here? I wonder why this wasn't the implementation from the way go. Maybe we had a lot of issues with our own resolver, so it was better to trust esbuild here. |
I think it could be. But it's more consistent so I think it's worth to try it, too. |
This PR breaks this https://stackblitz.com/edit/vitejs-vite-rguug6?file=package.json&terminal=dev. |
Closing as #8714 is merged and this PR needs to solve the following things:
|
Hi @sapphi-red I have performed a rebase of #8706 on top of main branch, the issue went away. Same thing for #8709, I tested against the stackblitz project and dequal gives me true without crashing the page. |
The time spent running test suits on CI before and after these changes seems similar. I am not sure if the test suits go though the optimize-deps phase. |
Description
fixes #8702
fixes #8704
Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).