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

Include base in hotFile without modifying server.origin replacement #296

Merged
merged 2 commits into from
May 17, 2024

Conversation

danielztolnai
Copy link
Contributor

@danielztolnai danielztolnai commented May 16, 2024

Unfortunately #290 - while fixing the hotFile problem - broke the transform function's origin replacement when using a base in the configuration resulting in broken asset URLs.

Vite generates URLs using the origin and the base. As this plugin now replaces the origin (__laravel_vite_placeholder__) by default with the origin+base, the base is included twice in the asset URLs, which results in the given assets not loading.

On the other hand, the original problem that #290 solved was real as well, as Laravel did not take the base into account before this PR and the main files did not load when setting base in the configuration. This means that the base has to be included in the hotFile. It should also be included when setting the origin explicitly, which is a special case that #290 did not handle.

This pull request includes the base in the hotFile, but leaves the origin (or more precisely the replacement) as is. This should both solve the original problem and avoid including the base in the asset URLs twice.

before after
public/hot content https://test.localhost:443/vite-test-base https://test.localhost:443/vite-test-base
asset URL example https://test.localhost/vite-test-base/vite-test-base/resources/images/test.svg https://test.localhost/vite-test-base/resources/images/test.svg
chunk URL example https://test.localhost/vite-test-base/node_modules/.vite/deps/chunk-NDQ3I6BD.js?v=df756ebe https://test.localhost/vite-test-base/node_modules/.vite/deps/chunk-NDQ3I6BD.js?v=df756ebe

This change should not be a breaking change as it is a fix to something that changed in vite 5 compared to vite 4 as mentioned here.

Copy link
Member

@timacdonald timacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@driesvints driesvints merged commit 5162674 into laravel:1.x May 17, 2024
3 checks passed
@driesvints
Copy link
Member

Thanks! We'll try to get this one out as soon as we're able.

@driesvints
Copy link
Member

Released!

@danielztolnai danielztolnai deleted the fix/base-included-twice branch May 28, 2024 08:26
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.

None yet

4 participants