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

Switch default asset rule to 'asset' from 'asset/resource' #419

Open
Judahmeek opened this issue Feb 12, 2024 · 3 comments
Open

Switch default asset rule to 'asset' from 'asset/resource' #419

Judahmeek opened this issue Feb 12, 2024 · 3 comments

Comments

@Judahmeek
Copy link
Contributor

@G-Rath @tomdracz Can either of you think of any possible issues with changing Shakapacker's default asset rule to use asset instead of asset/resource?

According to Webpack's documentation on asset modules rules, this would inline smaller assets, reducing the number of fetch cals from the client in exchange for a slight increase in bundle size.

@G-Rath
Copy link
Contributor

G-Rath commented Feb 13, 2024

I've not got a lot of experience with asset serving via webpack but happy to dig further into it; though generally the idea is that it's better to have more requests for smaller assets so that they can be cached independently, and that HTTP/2+ helps enable this by lowering the cost of repeated connections (via TCP connection reuse).

In practice HTTP/2+ is still not universal enough to start saying "stop bundling everything into a single big blob", but it's enough that it's worth considering (this is one of the "advantages" of importmaps...).

I think ultimately it's probably worth making an option so people can optimize based on their expected audience - I'll have a play around with some of our apps to see what kind of difference this makes

@tomdracz
Copy link
Collaborator

tomdracz commented Feb 13, 2024

I've tested this in a toy repo and can't see any obvious breakage. Effectively it just inlines things that are smaller than 8kb (I think).

Agree with @G-Rath on "more requests are better" bit, generally. With the sizes we're talking about here, it might not be the biggest problem in the world though.

My preference would be probably leave the default as is, but document and allow change through opt-in if one wants to fine-tune the performance. The latter might lead us nicely back to #80

@Judahmeek
Copy link
Contributor Author

@justin808 what's your opinion, given G-Rath & Tom's feedback?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants