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

typeof window === "undefined" not simplified when --define:window=undefined #1407

Closed
Jarred-Sumner opened this issue Jun 29, 2021 · 2 comments

Comments

@Jarred-Sumner
Copy link
Contributor

Jarred-Sumner commented Jun 29, 2021

Command:

esbuild input.js --define:window=undefined --minify

input.js:

if (typeof window !== "undefined") {
  console.log("this code should not exist");
} else {
  console.log("this code should exist");
}

Output:

typeof undefined!="undefined"?console.log("this code should not exist"):console.log("this code should exist");

It works as expected if you replace window with undefined.

input.js:

if (typeof undefined !== "undefined") {
  console.log("this code should not exist");
} else {
  console.log("this code should exist");
}

Output:

console.log("this code should exist");

It also works as expected if you set define to null

Command:

esbuild input.js --define:window=null --minify

input.js:

if (typeof window !== "undefined") {
  console.log("this code should not exist");
} else {
  console.log("this code should exist");
}

Output:

console.log("this code should not exist");

Different since typeof null === 'object'

I noticed this first in my bundler's implementation, realized it was reproducible in esbuild as well and wanted to give you a heads up.

This particular example -- typeof window is common for running separate code in Node/web workers. Sometimes people hardcode it this way in their webpack config to skip requiring backend-only code. See the example in Webpack's documentation

In webpack's case, I think people usually make it "typeof window": "undefined" so that they theoretically could still have a window variable in the code somewhere. That wouldn't work here since it's parsed as JSON which lacks a typeof operator, but it probably doesn't matter in practice

@Jarred-Sumner Jarred-Sumner changed the title typeof window === "undefined" not simplified when --define:window=undefined typeof window === "undefined" not simplified when --define:window=undefined Jun 29, 2021
@evanw
Copy link
Owner

evanw commented Jun 30, 2021

Yes, I've also noticed this before as well. The problem is that undefined and Infinity and NaN are implemented using the "define" feature and define values are not substituted in chains. I'm not sure that chain substitution of defines is a good idea, so I'm not immediately sure how to fix this. It could be nice to fix though.

@Jarred-Sumner
Copy link
Contributor Author

Jarred-Sumner commented Jul 2, 2021

As an extreme, with simplifying undefined from defines enabled in the new bundler and using the bundler's JS runtime, compared to esbuild's output, the same source code runs 2.3x faster.

A lot of this is due to JavaScriptCore starting up faster than V8 (unrelated to esbuild), but part of that is because it doesn't have to parse "react-dom". Note that in this case, spjs is also running the TSX parser, printing the code, and doesn't remove whitespace or shorten symbol names, so Node should be at a slight advantage by default. Also that the current integration copies source code strings multiple times due to issues with the public-facing JavaScriptCore C API (I need to make it use the C++ API)

CleanShot 2021-07-01 at 21 26 55@2x

Source files
    // index.tsx (sorry for bad spacing)
    import { Main } from "./main";
    import classNames from "classnames";
    const Base = ({}) => {
    const name =
        typeof location !== "undefined"
        ? decodeURIComponent(location.search.substring(1))
        : null;
    return <Main productName={name || "Bundler"} />;
    };

    function startReact() {
    const ReactDOM = require("react-dom");
    ReactDOM.render(<Base />, document.querySelector("#reactroot"));
    }

    if (typeof window !== "undefined") {
    console.log("HERE!!");
    globalThis.addEventListener("DOMContentLoaded", () => {
        startReact();
    });

    startReact();
    } else {
    const ReactDOMServer = require("react-dom/server.browser");
    console.log(ReactDOMServer.renderToString(<Base />));
    }

    export { Base };

    // main.tsx
    export const Main = ({ productName }) => {
    return (
        <>
        <header>
            <div className="Title">CSS HMR Stress Test</div>
            <p className="Description">
            This page visually tests how quickly a bundler can update CSS over Hot
            Module Reloading.
            </p>
        </header>
        <main className="main">
            <section className="ProgressSection">
            <p className="Subtitle">
                <span className="Subtitle-part">
                Ran: <span className="timer"></span>
                </span>
            </p>

            <div className="ProgressBar-container">
                <div className="ProgressBar"></div>
            </div>
            <div className="SectionLabel">
                The progress bar should move from left to right smoothly.
            </div>
            </section>

            <section>
            <div className="Spinners">
                <div className="Spinner-container Spinner-1">
                <div className="Spinner"></div>
                </div>

                <div className="Spinner-container Spinner-2">
                <div className="Spinner"></div>
                </div>

                <div className="Spinner-container Spinner-3">
                <div className="Spinner"></div>
                </div>

                <div className="Spinner-container Spinner-4">
                <div className="Spinner"></div>
                </div>
            </div>
            <div className="SectionLabel">
                The spinners should rotate &amp; change color smoothly.
            </div>
            </section>
        </main>
        <footer>
            <div className="SectionLabel FooterLabel">
            There are no CSS animations on this page.
            </div>

            <div className="Bundler-container">
            <div className="Bundler">{productName}</div>
            <div className="Bundler-updateRate">
                Saving a css file every&nbsp;
                <span className="highlight">
                <span className="interval"></span>ms
                </span>
            </div>
            </div>
        </footer>
        </>
    );
    };

esbuild command:

esbuild --bundle src/index.tsx --outfile=out.esbuild.js --platform=node --inject:react-inject.js --minify

@evanw evanw closed this as completed in 54cfb1e Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants