vite icon indicating copy to clipboard operation
vite copied to clipboard

feat: support callback for `build.assetsInlineLimit`

Open seivan opened this issue 3 years ago • 6 comments

Description

  • [x] assetsInlineLimit can now accept a callback for opting into inlining on a case by case basis.
  • [x] Passes the filePath: string, contentSize: number and currently accrued bundled size totalBundledSize: number.
  • [x] Using the total base64 size as opposed to file-size for size.
  • [x] Changed default size from 4kb to 6kb so current inlined builds won't change.
  • [x] User can choose to whitelist, blacklist, filter on base64 size or total bundled size up til now.
  • [x] Tests are green and changed playground/css/vite.config-relative-base.js to use the callback.
  • [x] Fixes: https://github.com/vitejs/vite/issues/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?

  • [x] New Feature

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

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

seivan avatar Jun 22 '22 12:06 seivan

Deploy Preview for vite-docs-main ready!

Name Link
Latest commit 2eef2a8b3ada0ae4f9a7a464e4956de6b2628fa3
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...

QR Code

Use your smartphone camera to open QR code link.

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

netlify[bot] avatar Jun 22 '22 12:06 netlify[bot]

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

Benjamin1333 avatar Jul 07 '22 09:07 Benjamin1333

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 https://github.com/vitejs/vite/pull/2909 - should SVG be under the inline assets umbrella without base64?

seivan avatar Aug 02 '22 02:08 seivan

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.

bluwy avatar Aug 02 '22 05:08 bluwy

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

haikyuu avatar Nov 03 '22 08:11 haikyuu

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

ArnaudBarre avatar Dec 16 '23 20:12 ArnaudBarre