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

bug: minifier used to transpile pollutes global scope with helper methods #139

Closed
Oksydan opened this issue Mar 29, 2021 · 11 comments · Fixed by #306
Closed

bug: minifier used to transpile pollutes global scope with helper methods #139

Oksydan opened this issue Mar 29, 2021 · 11 comments · Fixed by #306
Labels
bug Something isn't working

Comments

@Oksydan
Copy link

Oksydan commented Mar 29, 2021

Bug description

Hi!
I am using esbuild-loader for my project with jQuery (jQuery is an external) and bootstrap 4.
While webpack is creating chunks for dynamic imports with production build, it crates weird variable assign.
I am importing bootstrap dropdown dynamically on action. In minified chunk file it creates assing var $=Object.getOwnPropertySymbols with global scope. Which brakes my jQuery after file being imported.

I made minification with TerserWebpackPlugin and everything works fine.

Reproduction steps

I made repository with simple webpack config to reproduce the problem. https://github.com/Oksydan/esbuild-loader-bug
Chunk with id 3 is made from bootstrap dropdown. You can find there that var $=Object.getOwnPropertySymbols.

Environment

  • esbuild-loader version: 2.9.1
  • Webpack version: 4.46.0
  • Operating System: macOS Catalina 10.15.7
  • Node version: 15.5.1
  • Package manager (npm/yarn/pnpm) and version: npm 7.7.4
@floydjones1
Copy link

I don't know the exact issue but I want to say that esbuild minification under the hook isn't completely stable right now, and your problem might be fixed if this package updates to the latest esbuild compiler. But I would recommend that if you find a version of esbuild-loader that works for you do not upgraded until esbuild reaches a stable version 1.0.0

@privatenumber
Copy link
Owner

privatenumber commented Apr 11, 2021

@Oksydan

I had to install popper.js to get the build to work, and I had to create a run-time environment to fully test, but it seems working fine to me.

I created an index.html file in dist:

<!DOCTYPE html>
<html lang="en">
<head>
	<script src="https://unpkg.com/jquery@3.6.0/dist/jquery.js"></script>
	<script src="./js/index.js"></script>
</head>
<body>
</body>
</html>

and spun it up with npx http-server dist.

Can you double check? And if it's still happening, please provide more reproduction steps and paste the error stack.

@joshwilsonvu
Copy link

joshwilsonvu commented Apr 14, 2021

I can confirm that this is a problem, it can show up when ESBuildMinifyPlugin is run on multiple bundles somehow (chunk splitting, multiple entry points, whatever). The problem isn't really that it interferes with jQuery—that's a byproduct—it's that ESBuild writes helpers with var X = at the top level scope.

This is fine for modules, but for scripts (bundles) that scope is the global scope. ESBuild's transform API is only meant to run on modules. Anyone shipping only one bundle will be unaffected by this global scope pollution, but anyone using multiple bundles can run into a situation where the helper variables in each asset conflict, and since they are declared with var, they can silently overwrite each other.

Example:

// bundle 1
var et = Object.defineProperty;
var ke = Object.prototype.hasOwnProperty;
var Fe = Object.getOwnPropertySymbols,
  $e = Object.prototype.propertyIsEnumerable;
var qe = (s, o, e) => (o in s ? et(s, o, { enumerable: !0, configurable: !0, writable: !0, value: e }) : (s[o] = e)),
  V = (s, o) => {
    for (var e in o || (o = {})) ke.call(o, e) && qe(s, e, o[e]);
    if (Fe) for (var e of Fe(o)) $e.call(o, e) && qe(s, e, o[e]);
    return s;
  };
  /* ... */

// bundle 2
var _t = Object.defineProperty;
var He = Object.prototype.hasOwnProperty;
var ue = Object.getOwnPropertySymbols,
  Fe = Object.prototype.propertyIsEnumerable;
var Xe = (n, r, e) => (r in n ? _t(n, r, { enumerable: !0, configurable: !0, writable: !0, value: e }) : (n[r] = e)),
  ee = (n, r) => {
    for (var e in r || (r = {})) He.call(r, e) && Xe(n, e, r[e]);
    if (ue) for (var e of ue(r)) Fe.call(r, e) && Xe(n, e, r[e]);
    return n;
  };
  /* ... */

Notice how Fe is assigned twice, to different values each time? We have to avoid polluting the global scope, or things will break in very unexpected and difficult to debug ways.

My recommendation for script assets is to wrap ESBuild's full transform output in an IIFE. We'll have to use magic-string or something to keep the source maps accurate. If we can detect that certain Webpack assets are ES6 modules (does Webpack ever do that?), the problem is already resolved and we can't wrap the asset in an IIFE.

I'm willing to help contribute a PR for this. I'd love to get a fixed merged quickly so I can keep using this plugin. Thanks for all your work on it.

@privatenumber
Copy link
Owner

Yes, please provide a minimal reproduction.

The above reproduction seems to be working fine for me.

@joshwilsonvu
Copy link

Sorry, I meant "PR", not "MR". I will spin up a repository to try to reproduce, but keep in mind it's only a problem when ESBuild generates helper variables with the same name that are assigned different values, and while that should be deterministic it may be hard to reproduce on purpose.

@joshwilsonvu
Copy link

Good news, I found a simple workaround! Using

new ESBuildMinifyPlugin({
  ...otherOptions,
  format: 'iife'
})

wraps ESBuild's helpers in an IIFE to prevent these global variable conflicts. I personally would vouch for this to be the default, as it is ESBuild's default format when bundling for the web, but will leave that decision to the maintainers.

@Oksydan
Copy link
Author

Oksydan commented Apr 14, 2021

@joshwilsonvu I can confirm that solved my problem.
I would like to help and make better repository to reproduce the problem but at this moment I don't have enough time 😞

@privatenumber
Copy link
Owner

I was finally able to reproduce this.

This happens in situations where the minifier is used to transpile instead of the loader. The helpers are injected top-level outside of the module closures.

In this example, two output chunks declare the same variable d to the global scope:

Input

webpack.config.js

const path = require('path');
const { ESBuildMinifyPlugin } = require('esbuild-loader');

module.exports = {
	mode: 'production',

	entry: './src/index.js',

	output: {
		clean: true,
		path: path.resolve(__dirname, `./dist/`),
	},

	optimization: {
		minimizer: [
			new ESBuildMinifyPlugin({
				target: 'es2015',
			}),
		],
	},
};

src/a.js

export default async () => {};

src/b.js

console.log(1 ** 1);
(async () => {});
const y = {};
const {...z} = {...y};
try {
	throw new Error();
} catch {}
console.log(y?.d, y?.d ?? '');
Output

main.js

(()=>{var b={},g={};function r(e){var i=g[e];if(i!==void 0)return i.exports;var t=g[e]={exports:{}};return b[e](t,t.exports,r),t.exports}r.m=b,(()=>{var e=Object.getPrototypeOf?t=>Object.getPrototypeOf(t):t=>t.__proto__,i;r.t=function(t,o){if(o&1&&(t=this(t)),o&8||typeof t=="object"&&t&&(o&4&&t.__esModule||o&16&&typeof t.then=="function"))return t;var c=Object.create(null);r.r(c);var a={};i=i||[null,e({}),e([]),e(e)];for(var n=o&2&&t;typeof n=="object"&&!~i.indexOf(n);n=e(n))Object.getOwnPropertyNames(n).forEach(l=>a[l]=()=>t[l]);return a.default=()=>t,r.d(c,a),c}})(),(()=>{r.d=(e,i)=>{for(var t in i)r.o(i,t)&&!r.o(e,t)&&Object.defineProperty(e,t,{enumerable:!0,get:i[t]})}})(),(()=>{r.f={},r.e=e=>Promise.all(Object.keys(r.f).reduce((i,t)=>(r.f[t](e,i),i),[]))})(),(()=>{r.u=e=>""+e+".js"})(),(()=>{r.g=function(){if(typeof globalThis=="object")return globalThis;try{return this||new Function("return this")()}catch(e){if(typeof window=="object")return window}}()})(),(()=>{r.o=(e,i)=>Object.prototype.hasOwnProperty.call(e,i)})(),(()=>{var e={},i="minification-bug:";r.l=(t,o,c,a)=>{if(e[t]){e[t].push(o);return}var n,l;if(c!==void 0)for(var u=document.getElementsByTagName("script"),p=0;p<u.length;p++){var f=u[p];if(f.getAttribute("src")==t||f.getAttribute("data-webpack")==i+c){n=f;break}}n||(l=!0,n=document.createElement("script"),n.charset="utf-8",n.timeout=120,r.nc&&n.setAttribute("nonce",r.nc),n.setAttribute("data-webpack",i+c),n.src=t),e[t]=[o];var s=(h,m)=>{n.onerror=n.onload=null,clearTimeout(d);var _=e[t];if(delete e[t],n.parentNode&&n.parentNode.removeChild(n),_&&_.forEach(v=>v(m)),h)return h(m)},d=setTimeout(s.bind(null,void 0,{type:"timeout",target:n}),12e4);n.onerror=s.bind(null,n.onerror),n.onload=s.bind(null,n.onload),l&&document.head.appendChild(n)}})(),(()=>{r.r=e=>{typeof Symbol!="undefined"&&Symbol.toStringTag&&Object.defineProperty(e,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(e,"__esModule",{value:!0})}})(),(()=>{var e;r.g.importScripts&&(e=r.g.location+"");var i=r.g.document;if(!e&&i&&(i.currentScript&&(e=i.currentScript.src),!e)){var t=i.getElementsByTagName("script");t.length&&(e=t[t.length-1].src)}if(!e)throw new Error("Automatic publicPath is not supported in this browser");e=e.replace(/#.*$/,"").replace(/\?.*$/,"").replace(/\/[^\/]+$/,"/"),r.p=e})(),(()=>{var e={179:0};r.f.j=(o,c)=>{var a=r.o(e,o)?e[o]:void 0;if(a!==0)if(a)c.push(a[2]);else{var n=new Promise((f,s)=>a=e[o]=[f,s]);c.push(a[2]=n);var l=r.p+r.u(o),u=new Error,p=f=>{if(r.o(e,o)&&(a=e[o],a!==0&&(e[o]=void 0),a)){var s=f&&(f.type==="load"?"missing":f.type),d=f&&f.target&&f.target.src;u.message="Loading chunk "+o+` failed.
(`+s+": "+d+")",u.name="ChunkLoadError",u.type=s,u.request=d,a[1](u)}};r.l(l,p,"chunk-"+o,o)}};var i=(o,c)=>{var[a,n,l]=c,u,p,f=0;for(u in n)r.o(n,u)&&(r.m[u]=n[u]);if(l)var s=l(r);for(o&&o(c);f<a.length;f++)p=a[f],r.o(e,p)&&e[p]&&e[p][0](),e[a[f]]=0},t=self.webpackChunkminification_bug=self.webpackChunkminification_bug||[];t.forEach(i.bind(null,0)),t.push=i.bind(null,t.push.bind(t))})();var w={};r.e(85).then(r.bind(r,85)),r.e(326).then(r.t.bind(r,326,23))})();

85.js

var d=(f,i,n)=>new Promise((s,t)=>{var e=u=>{try{c(n.next(u))}catch(a){t(a)}},l=u=>{try{c(n.throw(u))}catch(a){t(a)}},c=u=>u.done?s(u.value):Promise.resolve(u.value).then(e,l);c((n=n.apply(f,i)).next())});(self.webpackChunkminification_bug=self.webpackChunkminification_bug||[]).push([[85],{85:(f,i,n)=>{"use strict";n.r(i),n.d(i,{default:()=>s});const s=()=>d(this,null,function*(){})}}]);

326.js

var d=Object.defineProperty;var h=Object.prototype.hasOwnProperty;var e=Object.getOwnPropertySymbols,r=Object.prototype.propertyIsEnumerable,u=Math.pow,b=(n,c,o)=>c in n?d(n,c,{enumerable:!0,configurable:!0,writable:!0,value:o}):n[c]=o,f=(n,c)=>{for(var o in c||(c={}))h.call(c,o)&&b(n,o,c[o]);if(e)for(var o of e(c))r.call(c,o)&&b(n,o,c[o]);return n};var g=(n,c)=>{var o={};for(var i in n)h.call(n,i)&&c.indexOf(i)<0&&(o[i]=n[i]);if(n!=null&&e)for(var i of e(n))c.indexOf(i)<0&&r.call(n,i)&&(o[i]=n[i]);return o};var k=(n,c,o)=>new Promise((i,l)=>{var w=s=>{try{t(o.next(s))}catch(a){l(a)}},p=s=>{try{t(o.throw(s))}catch(a){l(a)}},t=s=>s.done?i(s.value):Promise.resolve(s.value).then(w,p);t((o=o.apply(n,c)).next())});(self.webpackChunkminification_bug=self.webpackChunkminification_bug||[]).push([[326],{326:()=>{var o;console.log(u(1,1));const n={},c=g(f({},n),[]);try{throw new Error}catch(i){}console.log(n==null?void 0:n.d,(o=n==null?void 0:n.d)!=null?o:"")}}]);

@privatenumber privatenumber changed the title ESBuildMinifyPlugin minification problem bug: minifier used to transpile pollutes global scope with helper methods May 1, 2021
@privatenumber privatenumber added the bug Something isn't working label May 1, 2021
@crutch12
Copy link

crutch12 commented May 12, 2022

@privatenumber will you fix it? It's horrible, I've got intersection with window._ (lodash)!

Generated minified code (by ESBuildMinifyPlugin for { children, ...props } expression)

var _ = (r,p)=>{
    var n = {};
    for (var o in r)
        L.call(r, o) && p.indexOf(o) < 0 && (n[o] = r[o]);
    if (r != null && f)
        for (var o of f(r))
            p.indexOf(o) < 0 && W.call(r, o) && (n[o] = r[o]);
    return n
}
;

How it used:

// ...
s = _(i, ["children"]) // { children, ...props }

Spent about 2 hours to get what's going on.

Fixed by ejecting lodash from window:

{
  test: /\.jsx?$/,
  parser: { amd: false },
},

So... window._ returns generated by ESBuildMinifyPlugin function. I think it's not okay

image

UPD: Sorry, I've missed this comment (#139 (comment))
With format: 'iife' everything works fine.

@tjmgregory
Copy link

@joshwilsonvu Our saviour, this had us pulling our hair out - thanks so much! Would certainly be in favour of this being the default for the library to save the next poor souls 👻

@crutch12
Copy link

crutch12 commented Jun 1, 2022

I've got another problem with format: 'iife' - it breaks Webpack Module Federation :D
#258

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants