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

Preload code not passing through rollup #9016

Closed
7 tasks done
NikolayMakhonin opened this issue Jul 10, 2022 · 13 comments · Fixed by #14231
Closed
7 tasks done

Preload code not passing through rollup #9016

NikolayMakhonin opened this issue Jul 10, 2022 · 13 comments · Fixed by #14231
Labels
breaking change p2-nice-to-have Not breaking anything but nice to have (priority) rollup plugin compat
Milestone

Comments

@NikolayMakhonin
Copy link

NikolayMakhonin commented Jul 10, 2022

Describe the bug

I added a rollup plugin to vite config, but it turned out that it does not affect the preload code. The bundle has mixed code - passed through rollup and not passed through rollup. Could you make preload code go through rollup too?

See the bundle:

dist/assets/index.02199377.js
// this code is not passed through rollup. const is not converted to var by rollup/babel plugin
const p = function polyfill() {
  const relList = document.createElement("link").relList;
  if (relList && relList.supports && relList.supports("modulepreload")) {
    return;
  }
  for (const link of document.querySelectorAll('link[rel="modulepreload"]')) {
    processPreload(link);
  }
  new MutationObserver((mutations) => {
    for (const mutation of mutations) {
      if (mutation.type !== "childList") {
        continue;
      }
      for (const node of mutation.addedNodes) {
        if (node.tagName === "LINK" && node.rel === "modulepreload")
          processPreload(node);
      }
    }
  }).observe(document, { childList: true, subtree: true });
  function getFetchOpts(script) {
    const fetchOpts = {};
    if (script.integrity)
      fetchOpts.integrity = script.integrity;
    if (script.referrerpolicy)
      fetchOpts.referrerPolicy = script.referrerpolicy;
    if (script.crossorigin === "use-credentials")
      fetchOpts.credentials = "include";
    else if (script.crossorigin === "anonymous")
      fetchOpts.credentials = "omit";
    else
      fetchOpts.credentials = "same-origin";
    return fetchOpts;
  }
  function processPreload(link) {
    if (link.ep)
      return;
    link.ep = true;
    const fetchOpts = getFetchOpts(link);
    fetch(link.href, fetchOpts);
  }
};
p();
var style = "";

// this code is passed through rollup. const is converted to var by rollup/babel plugin
var html = "<h1>Hello Vite!</h1>";
document.querySelector("#app").innerHTML = html;

Additional: If this issue will resolved, then this pull request is no longer needed.

Reproduction

see my repo here

npm create vite@latest
√ Select a framework: » vanilla
√ Select a variant: » vanilla
cd <vite project>
pnpm install
  • add babel dependencies
  • add babelrc config
  • add vite config
  • add rollup babel to the vite config
vite build --debug
  • see the dist/assets/index.[hash].js file

System Info

System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i7-3610QM CPU @ 2.30GHz
    Memory: 5.14 GB / 15.89 GB
  Binaries:
    Node: 16.15.1 - E:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.11.0 - E:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1023.0), Chromium (103.0.1264.49)
    Internet Explorer: 11.0.19041.906
  npmPackages:
    vite: ^2.9.9 => 2.9.14

Used Package Manager

pnpm

Logs

Logs
C:\Users\Admin\AppData\Roaming\npm\pnpm.cmd run build

> [email protected] build D:\projects\-temp\vite\vite-issues
> vite build --debug

  vite:config native esm config loaded in 261.08ms URL {
  href: 'file:///D:/projects/-temp/vite/vite-issues/vite.config.mjs',
  origin: 'null',
  protocol: 'file:',
  username: '',
  password: '',
  host: '',
  hostname: '',
  port: '',
  pathname: '/D:/projects/-temp/vite/vite-issues/vite.config.mjs',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
} +0ms
  vite:config using resolved config: {
  vite:config   build: {
  vite:config     target: [ 'es2019', 'edge88', 'firefox78', 'chrome87', 'safari13.1' ],
  vite:config     polyfillModulePreload: true,
  vite:config     outDir: '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: { plugins: [Array] },
  vite:config     minify: false,
  vite:config     terserOptions: {
  vite:config       module: true,
  vite:config       ecma: 5,
  vite:config       safari10: true,
  vite:config       mangle: false,
  vite:config       format: [Object]
  vite:config     },
  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   plugins: [
  vite:config     'vite:build-metadata',
  vite:config     'alias',
  vite:config     'vite:modulepreload-polyfill',
  vite:config     'vite:resolve',
  vite:config     'vite:html-inline-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-plugin-babel',
  vite:config     'vite:define',
  vite:config     'vite:css-post',
  vite:config     'vite:build-html',
  vite:config     'vite:worker-import-meta-url',
  vite:config     'vite:watch-package-data',
  vite:config     'commonjs',
  vite:config     'vite:data-uri',
  vite:config     'rollup-plugin-dynamic-import-variables',
  vite:config     'vite:asset-import-meta-url',
  vite:config     'babel',
  vite:config     'vite:build-import-analysis',
  vite:config     'vite:esbuild-transpile',
  vite:config     'vite:reporter',
  vite:config     'vite:load-fallback'
  vite:config   ],
  vite:config   configFile: 'D:/projects/-temp/vite/vite-issues/vite.config.mjs',
  vite:config   configFileDependencies: [ 'D:/projects/-temp/vite/vite-issues/vite.config.mjs' ],
  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   root: 'D:/projects/-temp/vite/vite-issues',
  vite:config   base: '/',
  vite:config   resolve: { dedupe: undefined, alias: [ [Object], [Object] ] },
  vite:config   publicDir: 'D:\\projects\\-temp\\vite\\vite-issues\\public',
  vite:config   cacheDir: 'D:\\projects\\-temp\\vite\\vite-issues\\node_modules\\.vite',
  vite:config   command: 'build',
  vite:config   mode: 'production',
  vite:config   isWorker: false,
  vite:config   isProduction: true,
  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     headers: 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   worker: {
  vite:config     format: 'iife',
  vite:config     plugins: [
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object], [Object], [Object],
  vite:config       [Object]
  vite:config     ],
  vite:config     rollupOptions: {}
  vite:config   }
  vite:config } +22ms
build.terserOptions is specified but build.minify is not set to use Terser. Note Vite now defaults to use esbuild for minification. If you still prefer Terser, set build.minify to "
terser".
vite v2.9.14 building for production...
✓ 4 modules transformed.
dist/assets/favicon.17e50649.svg   1.49 KiB
dist/index.html                    0.46 KiB
dist/assets/index.02199377.js      1.38 KiB / gzip: 0.57 KiB
dist/assets/index.38b0c734.css     0.19 KiB / gzip: 0.16 KiB

Process finished with exit code 0

Validations

@sapphi-red
Copy link
Member

Preload helper does not have a extension.

export const preloadHelperId = '\0vite/preload-helper'

So if you don't include '' to this extension config, @rollup/plugin-babel will skip the helper.

@NikolayMakhonin
Copy link
Author

Thanks, I added '' file extension to the babel config and it worked. But I'm not sure if this is not a workaround, perhaps the helper should have a js extension

@sapphi-red
Copy link
Member

IMO Vite should have js extension for helpers, too.

@bluwy
Copy link
Member

bluwy commented Jul 11, 2022

I'm not sure if helpers should have the .js extension, it's not necessary per the virtual modules convention as virtual modules are almost always JS. I think the '' file extension trick would be good for now and could use include to further narrow the virtual modules to transform.

But maybe the better way is to have @rollup/plugin-babel work on the output code instead, so it captures all code by default. That's what plugin-legacy does too.

@NikolayMakhonin
Copy link
Author

I think the babel of output code will generate more code because it will duplicate core-js modules and runtime code for all output files.

And I have two problems with @vitejs/plugin-legacy:

  1. I can't configure my own babel plugins set
  2. It not working with svelte-kit yet, see

@bluwy
Copy link
Member

bluwy commented Jul 11, 2022

  1. I can't configure my own babel plugins set

That's because plugin-legacy now only injects polyfills. Syntax transformation are handled by esbuild and build.target, down to es2015.

2. It not working with svelte-kit yet, see

SvelteKit had transitioned to just a Vite plugin, so perhaps it could work now too. That issue was written when SvelteKit had full control over Vite.

@NikolayMakhonin
Copy link
Author

  1. OK, I will learn this feature
  2. This is a fresh issue, see my comment here

@sapphi-red
Copy link
Member

I'm not sure if helpers should have the .js extension, it's not necessary per the virtual modules convention as virtual modules are almost always JS.

I've checked rollup/plugins repo. It seems most of the virtual modules has a extension.

https://github.com/rollup/plugins/blob/985cf4c422896ac2b21279f0f99db9d281ef73c2/packages/babel/src/constants.js#L8
https://github.com/rollup/plugins/blob/985cf4c422896ac2b21279f0f99db9d281ef73c2/packages/commonjs/src/helpers.js#L13-L14
https://github.com/rollup/plugins/blob/985cf4c422896ac2b21279f0f99db9d281ef73c2/packages/node-resolve/src/index.js#L16
https://github.com/rollup/plugins/blob/985cf4c422896ac2b21279f0f99db9d281ef73c2/packages/wasm/src/helper.ts#L3

I think this will make it more easy to handle these in general.
If we want to avoid .js when importing manually (for example, vite/modulepreload-polyfill for backend integration), we could resolve the id with .js:

export default function preloadPolyfillPlugin() {
  return {
    name: 'preload-polyfill-plugin',
    resolveId(id) {
      if (id === 'vite/modulepreload-polyfill') {
        return '\0vite/modulepreload-polyfill.js'
      }
    },
    load(id) {
      if (id === '\0vite/modulepreload-polyfill.js') {
        return // TODO
      }
    }
  }
}

@sapphi-red
Copy link
Member

That's because plugin-legacy now only injects polyfills. Syntax transformation are handled by esbuild and build.target, down to es2015.

plugin-legacy does transpile the code using babel and skips esbuild transpilation.

// @ts-ignore avoid esbuild transform on legacy chunks since it produces
// legacy-unsafe code - e.g. rewriting object properties into shorthands
opts.__vite_skip_esbuild__ = true
// @ts-ignore force terser for legacy chunks. This only takes effect if
// minification isn't disabled, because that leaves out the terser plugin
// entirely.
opts.__vite_force_terser__ = true
// @ts-ignore
// In the `generateBundle` hook,
// we'll delete the assets from the legacy bundle to avoid emitting duplicate assets.
// But that's still a waste of computing resource.
// So we add this flag to avoid emitting the asset in the first place whenever possible.
opts.__vite_skip_asset_emit__ = true
// @ts-ignore avoid emitting assets for legacy bundle
const needPolyfills =
options.polyfills !== false && !Array.isArray(options.polyfills)
// transform the legacy chunk with @babel/preset-env
const sourceMaps = !!config.build.sourcemap
const babel = await loadBabel()
const { code, map } = babel.transform(raw, {
babelrc: false,
configFile: false,
compact: !!config.build.minify,
sourceMaps,
inputSourceMap: sourceMaps ? chunk.map : undefined,
presets: [
// forcing our plugin to run before preset-env by wrapping it in a
// preset so we can catch the injected import statements...
[
() => ({
plugins: [
recordAndRemovePolyfillBabelPlugin(legacyPolyfills),
replaceLegacyEnvBabelPlugin(),
wrapIIFEBabelPlugin()
]
})
],
[
'env',
createBabelPresetEnvOptions(targets, {
needPolyfills,
ignoreBrowserslistConfig: options.ignoreBrowserslistConfig
})
]
]
})

And yes, plugin-legacy does not have an option to control babel finely.

@bluwy
Copy link
Member

bluwy commented Jul 11, 2022

I've checked rollup/plugins repo. It seems most of the virtual modules has a extension.

Interesting. I think it's still risky to expect virtual modules in general to have an extension though, but thanks for the pointer. Maybe we could consider adding extensions for Vite too.

plugin-legacy does transpile the code using babel and skips esbuild transpilation.

You're right, I forgot that plugin-legacy skips esbuild. In that case babel would be handling syntax transform then via plugin-legacy's target option.

@patak-dev patak-dev added this to the 5.0 milestone Apr 8, 2023
@patak-dev
Copy link
Member

Added this issue to the Vite 5 milestone. I think it may be a good idea to add an extension to all resolved internal virtual modules and see how things work there 👍🏼

@patak-dev
Copy link
Member

@sapphi-red would you explain why this would be a breaking change? Are downstream projects relying on the resolved id for our internal virtual modules?

@sapphi-red
Copy link
Member

I put that label in case someone didn't expect internal modules not to be transformed by transform hook. I don't know whether a project like that exists.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change p2-nice-to-have Not breaking anything but nice to have (priority) rollup plugin compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants