Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[0.6.x] Do not inline small assets, by default #131

Merged
merged 1 commit into from
Sep 15, 2022
Merged

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Aug 27, 2022

By default Vite will "inline" assets that are smaller than 4kb by base64 encoding them.

From a Laravel perspective this can cause issues for Blade apps that are looking to use the Vite::asset() helper on small assets.

Blade specific apps could set this to 0 manually, however I also find the inconsistent behaviour as the default strange - even if it is beneficial to SPA style applications. I feel setting this to 0 is a good default for Laravel, while still respecting the user configured value.

The alternative would be to add some magic into the asset helper, but I don't think we can generally assume the file will still be in it's pre-bundled location (e.g. resources/images/pixel.png) as apps may clear that stuff out for space reasons (e.g. Serverless).

Encoding at runtime also kinda sucks as well - and the inconsistency in the returned result is a little weird (plain URL vs base64 url).

I think if this is something the community wants we could add an additional function assetOrEncoded() or something along those lines. But I think we can punt that down the road for now.

Copy link
Member

@jessarcher jessarcher left a comment

Choose a reason for hiding this comment

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

Looks good. I'm curious why 0.6.0 though?

@timacdonald
Copy link
Member Author

timacdonald commented Aug 28, 2022

As it will change existing functionality from under the user, i.e. 4kb assets will now be in the build dir.

I'm happy to see it as a feature rather than a breaking if you think that would be better.

@jessarcher
Copy link
Member

I see it as a bug fix for a new Laravel feature

@timacdonald timacdonald changed the title [0.6.0] Do not inline small assets, by default [0.6.x] Do not inline small assets, by default Aug 28, 2022
@timacdonald
Copy link
Member Author

timacdonald commented Aug 28, 2022

Sounds good to me. We bumped to 6.0 with the last release so I can just leave this as is now.

@taylorotwell taylorotwell merged commit 41cc390 into main Sep 15, 2022
@taylorotwell taylorotwell deleted the asset-limit branch September 15, 2022 13:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants