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 variable dropped by mistake #1472

Closed
woody-li opened this issue Nov 10, 2023 · 9 comments · Fixed by #1504
Closed

Global variable dropped by mistake #1472

woody-li opened this issue Nov 10, 2023 · 9 comments · Fixed by #1504
Labels

Comments

@woody-li
Copy link

woody-li commented Nov 10, 2023

Bug report

Version 5.24.0

terser input
The List variable:

const List = ['a', 'b', 'c'];
function define(name) {
    return (value, { addInitializer }) =>
        addInitializer(function () {
            customElements.define(name, this);
        });
}
@define('x-button')
class Button {
    constructor(name) {
        this.ok = List.includes(name);
    }
}

terser output or error
The List variable dropped, and the reference still exists with List.

let h;
var p;
(p = 'x-button'),
    (e = (t, e) => {
        let { addInitializer: r } = e;
        return r(function () {
            customElements.define(p, this);
        });
    });
class g {
    static #t = ([h, t] = d(this, [], [e]).c);
    constructor(t) {
        this.ok = List.includes(t);
    }
    static #e = t();
}

output with unused=false
The List renamed as m, and reference convert as m as well.

const m = ['a', 'b', 'c', 'd'];
function y(t) {
    return (e, r) => {
        let { addInitializer: n } = r;
        return n(function () {
            customElements.define(t, this);
        });
    };
}
let b;
e = y('x-button');
class w {
    static #t = ([b, t] = p(this, [], [e]).c);
    constructor(t) {
        this.ok = m.includes(t);
    }
    static #e = t();
}

Expected result
Same as the output above without unused=false

@fabiosantoscode
Copy link
Collaborator

I got confused because the original code doesn't seem to be what's really going into Terser. But I was able to create this minimal repro from the example with unused=false:

const List = ['a', 'b', 'c', 'd'];
class Class {
    static prop = leak(this);
    method(t) {
        List.forEach(letter => console.log(letter));
    }
}

In this example List is dropped because Terser thinks Class is unused, but it does have a static prop that can make a call to method possible.

@woody-li
Copy link
Author

Yeah, I'm using decorator syntax to define custom elements.

The Class maybe seems like unused, so is it possibile compatible with this case?

@fabiosantoscode
Copy link
Collaborator

It may still work but it depends on what that function p does.

On Terser's end, List should be kept. I had a bunch of tries for this bug yesterday but I couldn't find anything that worked just right. Other cases get broken, or larger.

@phiresky
Copy link

phiresky commented Jan 19, 2024

I'm getting the same issue. This causes decorators to break with webpack. It seems convoluted but I hit it with some very simple real code:

@observer
class Wrapper extends React.Component {
	render(): React.ReactNode {
		return (
			<ErrorBoundary>
				<MainGUI />
			</ErrorBoundary>
		);
	}
}

Here's what webpack with swc or babel-loader make of it:

	var ErrorBoundary = __webpack_require__(83448);

	
	let _Wrapper;
	class Wrapper extends (main_React_Component1 = react.Component) {
		static #_ = ({
			c: [_Wrapper, main_initClass1]
		} = (0, _apply_decs_2203_r._)(this, [], [mobxreact_esm /* observer */.Pi], main_React_Component1));
		render() {
			return /*#__PURE__*/ react.createElement(
				ErrorBoundary /* default */.Z,
				null,
				/*#__PURE__*/ react.createElement(_MainGUI, null)
			);
		}
		static #_2 = main_initClass1();
	}

Runningt that code through webpack production build or terser (with default settings) removes the ErrorBoundary assignment and thus throws in the render function.

Here's my minimal example:

const importedfn = () => console.log("foo");

const cls = (() => {
	var ErrorBoundary = importedfn;

	let _Wrapper;
	class Wrapper {
		static _ = ((cls) => (_Wrapper = cls))(this);
		render() {
			ErrorBoundary("foobar");
		}
	}

	return _Wrapper;
})();

new cls().render();

npx terser minimal.js --unused=false --compress > minimal2.js

result:

const importedfn = () => console.log("foo"),
	cls = (() => {
		let _Wrapper;
		class Wrapper {
			static _ = ((cls) => {
				_Wrapper = this;
			})();
			render() {
				ErrorBoundary("foobar");
			}
		}
		return _Wrapper;
	})();
new cls().render();

where the ErrorBoundary definition is just fully gone. It throws ReferenceError: ErrorBoundary is not defined

@fabiosantoscode
Copy link
Collaborator

The JS examples here are fixed in 5.27.2.

I'm not sure about other examples using decorators. Because Terser doesn't compile any decorators, it depends on the output of the toolchain before it.

@woody-li
Copy link
Author

@fabiosantoscode thanks for your work.

I can reproduce the issue when there's a static field, and works fine without static.
example:

const List = ['a', 'b', 'c'];
//define function ignored.
@define('x-button')
class Button {
    static x = 1;
    constructor(name) {
        this.ok = List.includes(name);
    }
}

output as:

let c;
(e = [
    ((r = 'x-button'),
    (t, e) => {
        let { addInitializer: n } = e;
        return n(function () {
            customElements.define(r, this);
        });
    }),
]),
    new (class extends a {
        static #t = (() => {
            class r {
                static #t = ([c, t] = n(this, [], e).c);
                constructor(t) {
                    this.ok = List.includes(t);
                }
            }
        })();
        x = 1;
        constructor() {
            super(c), t();
        }
    })();

@fabiosantoscode
Copy link
Collaborator

Could you turn off Terser to see what's going into it @woody-li? Terser can't read decorators, and I'd like to see how they're compiled before they get into it.

I tried putting your input code through Babel, but I don't know your configuration (or whether you're using Babel)

@woody-li
Copy link
Author

I'm not sure about it reason, maybe something about webpack, not Terser.
I'll research more about it, and update info after that.

@fabiosantoscode
Copy link
Collaborator

fabiosantoscode commented Feb 21, 2024

If you set your optimization.minimizer option on webpack to false and compile again, you can see what terser would have seen, since it's the last on the chain

https://webpack.js.org/configuration/optimization/#optimizationminimizer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants