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

Global namespace pollution when using rollupOptions.output.format umd #17608

Open
7 tasks done
TomPradat opened this issue Jul 3, 2024 · 5 comments
Open
7 tasks done
Labels
has workaround p2-edge-case Bug, but has workaround or limited in scope (priority)

Comments

@TomPradat
Copy link

Describe the bug

I'm building a bundle with umd format as a target. When doing so I expect the result bundle to be wrapped in an iife function. The configuration looks like this :

  build: {
    rollupOptions: {
      output: {
        format: "umd",
        entryFileNames: "bundle.js",
      },
    },
  },

Instead, I have variables declared outside the iife :

image

This is a problem because the variables' declaration might override previous variables with the same name. This is how I encountered the issue.

Workaround :

So far, I have used a homemade plugin to wrap my final code in a iife :

import { defineConfig } from "vite";

// https://vitejs.dev/config/
export default defineConfig({
  build: {
    rollupOptions: {
      output: {
        format: "umd",
        entryFileNames: "bundle.js",
      },
    },
  },
  plugins: [viteWrapCodeInIIFE({ files: ["bundle.js"] })],
});

function viteWrapCodeInIIFE(options = {}) {
  return {
    name: "vite-wrap-code-in-iife",
    apply: "build",
    enforce: "post",
    generateBundle(outputOptions, bundle) {
      for (const [fileName, chunkOrAsset] of Object.entries(bundle)) {
        if (
          chunkOrAsset.type === "chunk" &&
          options.files &&
          options.files.includes(fileName)
        ) {
          chunkOrAsset.code = `(function () {${chunkOrAsset.code}})();`;
        }
      }
    },
  };
}

I'm willing to work on a PR. I believe the fix would be an extension of the #7948 so that is also applies when rollupOptions.output.format is umd or iife.

Reproduction

See steps

Steps to reproduce

  1. Bootstrap a vite project
npm init vite@latest test -- --template vanilla && cd test && npm install
  1. Install the "react-query" package that causes the build to have variables defined outside the iife
npm install @tanstack/react-query
  1. Create a vite.config.js with the umd format as a target
cat <<EOF > vite.config.js
import { defineConfig } from "vite";

// https://vitejs.dev/config/
export default defineConfig({
  build: {
    rollupOptions: {
      output: {
        format: "umd",
        entryFileNames: "bundle.js",
      },
    },
  },
});
EOF
  1. Change the main.js
cat <<EOF > main.js
import { QueryClient } from "@tanstack/react-query";

const queryClient = new QueryClient();
EOF
  1. Build the bundle
npm run build
  1. See the result in dist/bundle.js

System Info

System:
    OS: macOS 13.4.1
    CPU: (8) arm64 Apple M1 Pro
    Memory: 330.16 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.16.0 - ~/.nvm/versions/node/v18.16.0/bin/node
    Yarn: 3.5.1 - ~/.nvm/versions/node/v18.16.0/bin/yarn
    npm: 9.5.1 - ~/.nvm/versions/node/v18.16.0/bin/npm
    pnpm: 8.15.4 - ~/Library/pnpm/pnpm
    bun: 1.0.14 - ~/.bun/bin/bun
  Browsers:
    Brave Browser: 124.1.65.132
    Chrome: 126.0.6478.127
    Firefox Nightly: 121.0a1
    Safari: 16.5.2
  npmPackages:
    vite: ^5.3.1 => 5.3.3

Used Package Manager

npm

Logs

No response

Validations

@TomPradat TomPradat changed the title Global namespace pollution with when using rollupOptions.output.format umd Global namespace pollution when using rollupOptions.output.format umd Jul 3, 2024
@sapphi-red
Copy link
Member

I haven't checked deeply but I guess we should use opts.format === 'iife' || opts.format === 'umd' instead of config.build.lib for people using IIFE/UMD format without turning lib mode on.

if (config.build.lib) {

@TomPradat
Copy link
Author

I haven't checked deeply but I guess we should use opts.format === 'iife' || opts.format === 'umd' instead of config.build.lib for people using IIFE/UMD format without turning lib mode on.

if (config.build.lib) {

This is what I was thinking too 👍 . I can work on a PR as soon as this is confirmed as a bug

@sapphi-red sapphi-red added has workaround p2-edge-case Bug, but has workaround or limited in scope (priority) labels Jul 10, 2024
@sapphi-red
Copy link
Member

Yeah, I think it's a bug.

@syranide
Copy link

Even with format=IIFE (at least for non-libs) it seems that some level of global namespace pollution occurs:
#16443 (comment)

@eXaminator
Copy link

We also ran into this issue today because the _ variable was redefined and underscore didn't work anymore in our old'ish application. Thank you @TomPradat for the Workaround, it works like a charm! Would be great if this could be the default though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has workaround p2-edge-case Bug, but has workaround or limited in scope (priority)
Projects
None yet
Development

No branches or pull requests

4 participants