feat: support callback for build.assetsInlineLimit

[seivan]

Description

• assetsInlineLimit can now accept a callback for opting into inlining on a case by case basis.
• Passes the filePath: string, contentSize: number and currently accrued bundled size totalBundledSize: number.
• Using the total base64 size as opposed to file-size for size.
• Changed default size from 4kb to 6kb so current inlined builds won't change.
• User can choose to whitelist, blacklist, filter on base64 size or total bundled size up til now.
• Tests are green and changed playground/css/vite.config-relative-base.js to use the callback.
• Fixes: Add build.assetInlineExclude config #2173

Additional context

Testing!
I had a difficult time testing this locally, not sure how to link to my local fork using Yarn Berry (v3)
No matter file, link or portal prefixes used in package.json it couldn't find the vite executable.
Maybe someone could help out with that, so it'll be easier to try it out locally.

What I did instead was create a patch in yarn to test it out, but that's modifying the distributed javascript bundles.

Should close the PR if there is already a non intrusive way to inline on a case by case basis for, e.g FontFace woff2 files.

But I would love help on how to run vite locally referenced using Yarn/Berry for future reference if that's possible.

---

What is the purpose of this pull request?

• New Feature

Before submitting the PR, please make sure you do the following

• Read the Contributing Guidelines.
• Read the Pull Request Guidelines and follow the Commit Convention.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[netlify]

✅ Deploy Preview for vite-docs-main ready!

Name	Link
🔨 Latest commit	2eef2a8
🔍 Latest deploy log	https://app.netlify.com/sites/vite-docs-main/deploys/62c283c651f0c3000840f6fe
😎 Deploy Preview	https://deploy-preview-8717--vite-docs-main.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[Benjamin1333]

Yeah, great feature. Any infos about when this will be available?

[seivan]

Could I get an update on where this currently stands?
What do I need to change to move this forward?
If using a union is causing too much issue, I don't mind moving it into its own key, but my take on it is, that the current config is close to invalid on the basis that a fixed size doesn't take growth into account.
At least with file names there is a level of control, either way I was hoping this could have gone in before v3.

We have little to zero control over assets bundling now and it's starting to cause issues.

Edit: Also, what's the consensus on #2909 - should SVG be under the inline assets umbrella without base64?

[bluwy]

This is still in the project board for team discussion, which you can find at https://github.com/orgs/vitejs/projects/1/views/9. We haven't got around discussing it yet recently.

[yyx990803]

This is running filetoInlinedAsset on every asset file even if it doesn't end up get inlined, seems wasteful especially when there are many larger files that don't need to be inlined.

I think we should just use the raw buffer size like before.

[seivan]

@yyx990803 Fair enough, but being accurate with size is inherently important as many small add up.
However I got the same feedback internally and the current design is to just pass an object with various methods

So instead of passing three arguments, it's one argument with "heavy" stuff as computed on demand;

{
  name: string
  bufferSize: number
  size(): number
  totalSize(): number
}

And it isn't additional code/complexity either, which is nice.
Though I would want filePath in addition to name. Sometimes you're just looking for that one file, and other times it's all the files in a path.

Let the user decide if it's wasteful or not, at least Vite won't be doing it now unless opt-in, and even then you can choose to do it for e.g just assets/*

Will that work for you?

[patak-dev]

I think the object is a good idea, but I don't think we should expose size() here. It is a clever trick but we will be allowing users to easily reach for inefficient code. For most use cases, it is ok to use the file size and we should promote that.
I think we should have the object but with a plain size prop and not totalSize.

[seivan]

@patak-dev The problem is bufferSize doesn't match the REAL size and that is the concern this PR is trying to solve.

We have a bunch of assets, some should be inlined and some should not.
File name matters (explicit inlining)
Paths matter (glob inlining)
Size matters (It's too big, opt out for this one)
TotalSize (We're reaching our limit, stop inlining)

Now, all of these could inherently be done by the user in the callback, but it would cumbersome.

for instance, totalSize is very easy to do outside, just totalSize += realSize outside the callback.
But expecting the user to know and do

1. That the current size given to you is a lie
2. Getting the real size from said number

That being said, I've already written this for myself, so it's not a big deal.

Yeah I agree having an object there makes more sense for any further information needed to be passed down.
At the very least, using a fixed static size as a boolean to decide is broken so that would be fixed regardless of what data is sent.

Your call
👍🏼

That being said, I need help with a few things:

What's the consensus on #2909 - should SVG be under the inline assets umbrella without base64?

I might need some help resolving the conflict.

[patak-dev]

As you say, if someone needs the totalSize, it can be computed externally. I don't think it is common enough to justify having it in core.

And the same goes for the real size, we can be more clear in the docs and add a rationale about why the API is using the file size.

About #2909, I think it should but that PR should be merged first, no? We can already move with this PR before that IIUC.

Let me know if you try to resolve the conflict and you get blocked and I'll check it out 👍🏼

[haikyuu]

Any news on this PR? what stops it from being merged?

[ArnaudBarre]

For people following this PR, I've made a simpler scoped update in #15366