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

Minified global var names in conflict with other libraries using iife output format #7188

Closed
7 tasks done
thanoskrg opened this issue Mar 6, 2022 · 12 comments · Fixed by #7948
Closed
7 tasks done
Labels
feat: library mode p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@thanoskrg
Copy link

thanoskrg commented Mar 6, 2022

Describe the bug

Recently I came across an extreme edge case when vite would build and minify my library in iife format, but keep some global variables outside of the main closure. This resulted to one of these minified global vars to be named after ga which resulted to conflict with the actual google analytics library, causing my library to break during runtime.

This is an example of the output javascript I got:

code

My vite configuration is similar to the following one:

import path from "path";
import minifyHTML from "rollup-plugin-minify-html-literals";
import { defineConfig } from "vite";
import eslintPlugin from "vite-plugin-eslint";

export default defineConfig(() => {
  return {
    build: {
      outDir: "dist",
      emptyOutDir: false,
      sourcemap: "hidden",
      lib: {
        name: "MyLibrary",
        entry: path.resolve(__dirname, "MyLibrary/index.ts"),
        formats: ["iife"],
        fileName: () => "my-library.js",
      },
    },
    plugins: [eslintPlugin(), minifyHTML()],
  };
});

Expected behaviour

I would expect that globally defined variables that are part of my distributable code should be enclosed in the main closure when in iife format, to avoid such issues.

Temporary solution

What I've done to "hack" this for now and avoid any potential issues is that after my build finishes I run a custom node script which wraps the output javascript file in a closure likewise:

import fs from "fs";
import glob from "glob";
import path from "path";

const ENCODING = "utf8";
const outDir = path.resolve(__dirname, "../dist");

const searchFor = `${outDir}/my-library.js`;

function closurify(content: string) {
  return `(function(){${content}})();`; // do the hack
}

glob(searchFor, {}, (err, files) => {
  files.forEach((file) => {
    fs.readFile(file, { encoding: ENCODING }, (err, content) => {
      fs.writeFile(
        file,
        closurify(content),
        { encoding: ENCODING, flag: "w" }
      );
    });
  });
});

Reproduction [UPDATED]

https://github.com/thanoskrg/lit-playground

The issue appears when I use spread operator inside index.ts and it can be noticed in the output js on the first line.

System Info

System:
    OS: macOS 11.6
    CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
    Memory: 35.39 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.13.0 - ~/.nvm/versions/node/v16.13.0/bin/node
    Yarn: 1.22.17 - ~/.nvm/versions/node/v16.13.0/bin/yarn
    npm: 8.1.0 - ~/.nvm/versions/node/v16.13.0/bin/npm
  Browsers:
    Chrome: 98.0.4758.109
    Firefox: 97.0.1
    Safari: 14.1.2
  npmPackages:
    vite: ^2.8.0 => 2.8.0

Used Package Manager

npm

Logs

No response

Validations

@github-actions
Copy link

github-actions bot commented Mar 6, 2022

Hello @thanoskrg. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with need reproduction will be closed if they have no activity within 3 days.

@thanoskrg
Copy link
Author

I am under the impression that this issue does not need reproduction repository. I guess my issue is related to the way vite builds iife format outputs. I can provide an example repo but the output most probably will not yield this specific ga var name.

@Niputi should I provide reproduction url after all?

@Niputi
Copy link
Contributor

Niputi commented Mar 6, 2022

yes please do provide a reproduction anyway so we can know how you end up on in this situation

@thanoskrg
Copy link
Author

Ok @Niputi , I've updated the issue description with a reproduction repo url. Hope this helps.

@pd4d10
Copy link
Contributor

pd4d10 commented Apr 3, 2022

These seem to be the helper functions introduced by esbuild:

var __defProp = Object.defineProperty;
var __getOwnPropSymbols = Object.getOwnPropertySymbols;
var __hasOwnProp = Object.prototype.hasOwnProperty;
var __propIsEnum = Object.prototype.propertyIsEnumerable;
var __defNormalProp = (obj, key, value) => key in obj ? __defProp(obj, key, { enumerable: true, configurable: true, writable: true, value }) : obj[key] = value;
var __spreadValues = (a, b) => {
  for (var prop in b || (b = {}))
    if (__hasOwnProp.call(b, prop))
      __defNormalProp(a, prop, b[prop]);
  if (__getOwnPropSymbols)
    for (var prop of __getOwnPropSymbols(b)) {
      if (__propIsEnum.call(b, prop))
        __defNormalProp(a, prop, b[prop]);
    }
  return a;
};

@bluwy
Copy link
Member

bluwy commented Apr 4, 2022

This seems to be very similar to #5426, but for umd

@thanoskrg
Copy link
Author

This seems to be very similar to #5426, but for umd

Indeed @bluwy, they seem to be very similar issues. Is there any suggested solution for this? Is this suggestion the only way to go here?

@bluwy
Copy link
Member

bluwy commented Apr 5, 2022

For now that can be used as a workaround. I haven't looked into why/how terser handle things differently than esbuild, ideally they should behave similarly.

@bluwy bluwy added pending triage bug p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Apr 5, 2022
@thanoskrg
Copy link
Author

For now that can be used as a workaround.

Unfortunately, using build.minify: "terser" does not resolve the issue at the moment. I guess I should wait for the fix to come at some point.

@sapphi-red
Copy link
Member

sapphi-red commented Apr 28, 2022

This was happening here.

const res = await transformWithEsbuild(code, chunk.fileName, {
...config.esbuild,
target: target || undefined,
...(minify
? {
minify,
treeShaking: true,
format: rollupToEsbuildFormatMap[opts.format]
}
: undefined)
})
return res

esbuild injects utility function at the top of script like this.
You can see variables other than MyLibrary.
After minify, those variable names gets minified.

I am not sure how to fix this though.

@tobiasdalhof
Copy link

This was happening here.

const res = await transformWithEsbuild(code, chunk.fileName, {
...config.esbuild,
target: target || undefined,
...(minify
? {
minify,
treeShaking: true,
format: rollupToEsbuildFormatMap[opts.format]
}
: undefined)
})
return res

esbuild injects utility function at the top of script like this. You can see variables other than MyLibrary. After minify, those variable names gets minified.

I am not sure how to fix this though.

Why are they doing that tho? I was also having issues because of this.

@patak-dev
Copy link
Member

What do you think about injecting the esbuild helpers by hand inside the iife after esbuild is done when this is the target?
Something like this could work:

res = res.replace(
  /(.*)(var [^\s]+=function\(r\){"use strict";)(.*)/,
  (match, p1, p2, p3) => p2 + p1 + p3
);

Maybe it is good enough? I can't think of another fix that doesn't involve other options in esbuild here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: library mode p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants