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

Vite inlines icon resources that should not be inlined #6741

Closed
7 tasks done
dantman opened this issue Feb 3, 2022 · 4 comments · Fixed by #14958
Closed
7 tasks done

Vite inlines icon resources that should not be inlined #6741

dantman opened this issue Feb 3, 2022 · 4 comments · Fixed by #14958
Labels
enhancement New feature or request feat: html

Comments

@dantman
Copy link

dantman commented Feb 3, 2022

Describe the bug

When Vite asset-ifies <link> resources it applies the default assetsInlineLimit for JS without considering whether it makes sense to inline the resource in HTML.

One issue this causes is in site icons. If a site icon is small enough, instead of just putting it in /assets with a cache buster (which can be desirable for site icons), Vite will inline it into the document.

This is a problem since now site icons are downloaded inline with every page request which is problematic for a number of reasons:

  • Site icons are a low priority but now must be downloaded before critical page content
  • The data now comes that comes with every page load on a likely uncacheable request instead of allowing the browser to cache the icon
  • All site icons are downloaded, even if its an icon the browser would not have downloaded (unsupported file type, media query, unused size)

Manifests are also inlined, but that bug was reported in #5962.

Reproduction

https://github.com/dantman/vite-excessive-inline-resources-bug

System Info

System:
    OS: Linux 5.10 Ubuntu 21.10 21.10 (Impish Indri)
    CPU: (24) x64 AMD Ryzen 9 3900X 12-Core Processor
    Memory: 15.18 GB / 24.99 GB
    Container: Yes
    Shell: 5.1.8 - /bin/bash
  Binaries:
    Node: 17.4.0 - ~/.nvm/versions/node/v17.4.0/bin/node
    Yarn: 1.22.11 - ~/.nvm/versions/node/v17.4.0/bin/yarn
    npm: 8.3.1 - ~/.nvm/versions/node/v17.4.0/bin/npm
  npmPackages:
    vite: ^2.7.13 => 2.7.13

Used Package Manager

npm

Logs

vite:config no config file found. +0ms
  vite:config using resolved config: {
  vite:config   root: '/mnt/c/Users/danie/vite-test/vite-project',
  vite:config   base: '/',
  vite:config   mode: 'production',
  vite:config   configFile: undefined,
  vite:config   logLevel: undefined,
  vite:config   clearScreen: undefined,
  vite:config   build: {
  vite:config     target: [ 'es2019', 'edge88', 'firefox78', 'chrome87', 'safari13.1' ],
  vite:config     polyfillModulePreload: true,
  vite:config     outDir: '/mnt/c/Users/danie/vite-test/vite-project/dist',
  vite:config     assetsDir: 'assets',
  vite:config     assetsInlineLimit: 4096,
  vite:config     cssCodeSplit: true,
  vite:config     cssTarget: [ 'es2019', 'edge88', 'firefox78', 'chrome87', 'safari13.1' ],
  vite:config     sourcemap: false,
  vite:config     rollupOptions: { input: '/mnt/c/Users/danie/vite-test/vite-project/index.html' },
  vite:config     minify: 'esbuild',
  vite:config     terserOptions: {},
  vite:config     write: true,
  vite:config     emptyOutDir: null,
  vite:config     manifest: false,
  vite:config     lib: false,
  vite:config     ssr: false,
  vite:config     ssrManifest: false,
  vite:config     reportCompressedSize: true,
  vite:config     chunkSizeWarningLimit: 500,
  vite:config     watch: null,
  vite:config     commonjsOptions: { include: [Array], extensions: [Array] },
  vite:config     dynamicImportVarsOptions: { warnOnError: true, exclude: [Array] }
  vite:config   },
  vite:config   configFileDependencies: [],
  vite:config   inlineConfig: {
  vite:config     root: undefined,
  vite:config     base: undefined,
  vite:config     mode: undefined,
  vite:config     configFile: undefined,
  vite:config     logLevel: undefined,
  vite:config     clearScreen: undefined,
  vite:config     build: {}
  vite:config   },
  vite:config   resolve: { dedupe: undefined, alias: [ [Object], [Object] ] },
  vite:config   publicDir: '/mnt/c/Users/danie/vite-test/vite-project/public',
  vite:config   cacheDir: '/mnt/c/Users/danie/vite-test/vite-project/node_modules/.vite',
  vite:config   command: 'build',
  vite:config   isProduction: true,
  vite:config   plugins: [
  vite:config     'alias',
  vite:config     'vite:modulepreload-polyfill',
  vite:config     'vite:resolve',
  vite:config     'vite:html-inline-script-proxy',
  vite:config     'vite:css',
  vite:config     'vite:esbuild',
  vite:config     'vite:json',
  vite:config     'vite:wasm',
  vite:config     'vite:worker',
  vite:config     'vite:asset',
  vite:config     'vite:define',
  vite:config     'vite:css-post',
  vite:config     'vite:watch-package-data',
  vite:config     'vite:build-html',
  vite:config     'commonjs',
  vite:config     'vite:data-uri',
  vite:config     'rollup-plugin-dynamic-import-variables',
  vite:config     'vite:asset-import-meta-url',
  vite:config     'vite:build-import-analysis',
  vite:config     'vite:esbuild-transpile',
  vite:config     'vite:reporter',
  vite:config     'vite:load-fallback'
  vite:config   ],
  vite:config   server: {
  vite:config     preTransformRequests: true,
  vite:config     fs: { strict: true, allow: [Array], deny: [Array] }
  vite:config   },
  vite:config   preview: {
  vite:config     port: undefined,
  vite:config     strictPort: undefined,
  vite:config     host: undefined,
  vite:config     https: undefined,
  vite:config     open: undefined,
  vite:config     proxy: undefined,
  vite:config     cors: undefined
  vite:config   },
  vite:config   env: { BASE_URL: '/', MODE: 'production', DEV: false, PROD: true },
  vite:config   assetsInclude: [Function: assetsInclude],
  vite:config   logger: {
  vite:config     hasWarned: false,
  vite:config     info: [Function: info],
  vite:config     warn: [Function: warn],
  vite:config     warnOnce: [Function: warnOnce],
  vite:config     error: [Function: error],
  vite:config     clearScreen: [Function: clearScreen],
  vite:config     hasErrorLogged: [Function: hasErrorLogged]
  vite:config   },
  vite:config   packageCache: Map(0) { set: [Function (anonymous)] },
  vite:config   createResolver: [Function: createResolver],
  vite:config   optimizeDeps: {
  vite:config     esbuildOptions: { keepNames: undefined, preserveSymlinks: undefined }
  vite:config   }
  vite:config } +13ms
vite v2.7.13 building for production...
✓ 4 modules transformed.
dist/assets/apple-touch-icon.2d7ab2e6.png   13.82 KiB
dist/index.html                             4.60 KiB
dist/assets/index.f72635f1.js               0.85 KiB / gzip: 0.49 KiB
dist/assets/index.06d14ce2.css              0.17 KiB / gzip: 0.14 KiB

Validations

@YLivay
Copy link

YLivay commented Mar 23, 2022

We have a similar issues when bundling vite apps to electron/nwjs. there are some 3rd party integrations that expect urls/file paths and do not support inlined base64 assets.

@philwolstenholme
Copy link

I think #2173 (and #10578 and #8717) are related

@bluwy
Copy link
Member

bluwy commented Nov 23, 2022

I think it makes sense to not inline rel="icon" by default, and the same for manifest files too

@bluwy bluwy added enhancement New feature or request and removed pending triage labels Nov 23, 2022
@bluwy bluwy mentioned this issue Nov 23, 2022
7 tasks
@gautamkrishnar
Copy link

gautamkrishnar commented Jan 22, 2023

@bluwy It also makes sense to inline link rel="apple-touch-icon" by default since safari throws an error when the url is base64

eg:
Screenshot 2023-01-22 at 12 03 40 PM

@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request feat: html
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants