search

parcel-bundler / parcel

The zero configuration build tool for the web. 📦🚀
https://parceljs.org
MIT License
43.39k stars 2.27k forks source link

`scopeHoist: true` not being applied correctly when using the JS API #5826

Open danofpaco opened 3 years ago

danofpaco commented 3 years ago

🐛 bug report

The scope hoisting option appears to have ceased working.

🤔 Expected Behavior

The minimal, tidy output that I had been seeing...

😯 Current Behavior

My output now looks like:

(function(modules, entry, mainEntry, parcelRequireName, globalName) {
    /* eslint-disable no-undef */
    var globalObject =
        typeof globalThis !== 'undefined' ?
        globalThis :
        typeof self !== 'undefined' ?
        self :
        typeof window !== 'undefined' ?
        window :
        typeof global !== 'undefined' ?
        global :
        {};
    /* eslint-enable no-undef */

    // Save the require from previous bundle to this closure if any
    var previousRequire =
        typeof globalObject[parcelRequireName] === 'function' &&
        globalObject[parcelRequireName];

    var cache = previousRequire.cache || {};

// etc...

🔦 Context

Used from within a Node app for basic website bundling. One goal for me is minimal output. In project I have markup that may sometimes have an inline script or two, sometimes just a line or two of code. With scope hoisting, output was minimal - maybe just adding a closure around the lines. Without it, I'm seeing an extra 150 lines minimum due to all of the boilerplate that gets repeated - for each script on the page.

💻 Code Sample

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Hello World</title>
</head>
<body>
    <script>console.log('hello world')</script>
</body>
</html>
const Parcel = require('@parcel/core').default;
const defaultConfigContents = require('@parcel/config-default');
const defaultConfigFilePath = require.resolve('@parcel/config-default');
const parcelRcJson = require.resolve('./parcelrc.json');
const projectRoot = require('../cli/projectRoot');
const path = require('path');
const outDir = require('./outDir');

(async () => {
    await new Parcel({
        entries: path.join(projectRoot, './src/index.html'),
        config: parcelRcJson,
        defaultConfig: {
            ...defaultConfigContents,
            filePath: defaultConfigFilePath,
        },
        targets: {
            browser: {
                context: 'browser',
                distDir: outDir,
                optimize: process.env.NODE_ENV === 'production',
                isLibrary: false,
                outputFormat: 'global',
                publicUrl: './',
                scopeHoist: true, // <-- not working here
            },
        },
        disableCache: true,
        mode: process.env.NODE_ENV,
        // scopeHoist: true, <-- used to work here, but looks like the option moved under target instead
        logLevel: 'none',
        shouldPatchConsole: false,
        detailedReport: false,
    }).run();
})()

🌍 Your Environment

Software Version(s)
Parcel 2.0.0-nightly.579
Node 15.3.0
npm/Yarn npm 7.0.14
Operating System macOS 10.15.7
mischnic commented 3 years ago

This works for me:

const Parcel = require("@parcel/core").default;
const defaultConfigContents = require("@parcel/config-default");
const defaultConfigFilePath = require.resolve("@parcel/config-default");
const path = require("path");

(async () => {
    await new Parcel({
        entries: "index.js",
        defaultConfig: defaultConfigFilePath,
        targets: {
            browser: {
                distDir: "dist/",
                context: "browser",
                isLibrary: false,
                outputFormat: "global",
                publicUrl: "./",
                scopeHoist: false, // toggle and look at output file
            },
        },
        disableCache: true,
        mode: "production",
        shouldPatchConsole: false,
        detailedReport: false,
    }).run();
})();

Some toplevel options were recently moved into defaultTargetOptions: https://github.com/parcel-bundler/parcel/pull/5696

danofpaco commented 3 years ago

Given my example HTML file, and the settings you've shown (changing scopeHoist between true and false), this is the output I get regardless of the value for scopeHoist:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            // modules are defined as an array
            // [ module function, map of requires ]
            //
            // map of requires is short require name -> numeric require
            //
            // anything defined in a previous bundle is accessed via the
            // orig method which is the require for previous bundles

            (function(modules, entry, mainEntry, parcelRequireName, globalName) {
                /* eslint-disable no-undef */
                var globalObject =
                    typeof globalThis !== 'undefined' ?
                    globalThis :
                    typeof self !== 'undefined' ?
                    self :
                    typeof window !== 'undefined' ?
                    window :
                    typeof global !== 'undefined' ?
                    global :
                    {};
                /* eslint-enable no-undef */

                // Save the require from previous bundle to this closure if any
                var previousRequire =
                    typeof globalObject[parcelRequireName] === 'function' &&
                    globalObject[parcelRequireName];

                var cache = previousRequire.cache || {};
                // Do not use `require` to prevent Webpack from trying to bundle this call
                var nodeRequire =
                    typeof module !== 'undefined' &&
                    typeof module.require === 'function' &&
                    module.require.bind(module);

                function newRequire(name, jumped) {
                    if (!cache[name]) {
                        if (!modules[name]) {
                            // if we cannot find the module within our internal map or
                            // cache jump to the current global require ie. the last bundle
                            // that was added to the page.
                            var currentRequire =
                                typeof globalObject[parcelRequireName] === 'function' &&
                                globalObject[parcelRequireName];
                            if (!jumped && currentRequire) {
                                return currentRequire(name, true);
                            }

                            // If there are other bundles on this page the require from the
                            // previous one is saved to 'previousRequire'. Repeat this as
                            // many times as there are bundles until the module is found or
                            // we exhaust the require chain.
                            if (previousRequire) {
                                return previousRequire(name, true);
                            }

                            // Try the node require function if it exists.
                            if (nodeRequire && typeof name === 'string') {
                                return nodeRequire(name);
                            }

                            var err = new Error("Cannot find module '" + name + "'");
                            err.code = 'MODULE_NOT_FOUND';
                            throw err;
                        }

                        localRequire.resolve = resolve;
                        localRequire.cache = {};

                        var module = (cache[name] = new newRequire.Module(name));

                        modules[name][0].call(
                            module.exports,
                            localRequire,
                            module,
                            module.exports,
                            this
                        );
                    }

                    return cache[name].exports;

                    function localRequire(x) {
                        return newRequire(localRequire.resolve(x));
                    }

                    function resolve(x) {
                        return modules[name][1][x] || x;
                    }
                }

                function Module(moduleName) {
                    this.id = moduleName;
                    this.bundle = newRequire;
                    this.exports = {};
                }

                newRequire.isParcelRequire = true;
                newRequire.Module = Module;
                newRequire.modules = modules;
                newRequire.cache = cache;
                newRequire.parent = previousRequire;
                newRequire.register = function(id, exports) {
                    modules[id] = [
                        function(require, module) {
                            module.exports = exports;
                        },
                        {},
                    ];
                };

                Object.defineProperty(newRequire, 'root', {
                    get: function() {
                        return globalObject[parcelRequireName];
                    },
                });

                globalObject[parcelRequireName] = newRequire;

                for (var i = 0; i < entry.length; i++) {
                    newRequire(entry[i]);
                }

                if (mainEntry) {
                    // Expose entry point to Node, AMD or browser globals
                    // Based on https://github.com/ForbesLindesay/umd/blob/master/template.js
                    var mainExports = newRequire(mainEntry);

                    // CommonJS
                    if (typeof exports === 'object' && typeof module !== 'undefined') {
                        module.exports = mainExports;

                        // RequireJS
                    } else if (typeof define === 'function' && define.amd) {
                        define(function() {
                            return mainExports;
                        });

                        // <script>
                    } else if (globalName) {
                        this[globalName] = mainExports;
                    }
                }
            })({
                "3klNt": [function(require, module, exports) {
                    console.log('hello world');

                }, {}]
            }, ["3klNt"], "3klNt", "parcelRequire0bb0")

        </script>
    </body>
</html>
danofpaco commented 3 years ago

So, running some additional tests, this is what I'm seeing. It seems entirely dependent on there being a defaultTargetOptions present. The only way I was able to enable scope hoisting was for there to be a defaultTargetOptions: { shouldScopeHoist: true } - having scopeHoist: true on a target does nothing.

Here is what I'm seeing in terms of results for the combinations of undefined/true/false for shouldScopeHoist and scopeHoist:

shouldScopeHoist scopeHoist Did scope hoist?
true undefined yes
true true yes
true false no
undefined true no
undefined false no
undefined undefined no

So what I'm seeing is, the only way to enable scope hoisting is to set shouldScopeHoist to true in the defaultTargetOptions. Is this how it is intended to function?

So, given your example settings, I had to do the following for scope hoisting to work:

module.exports = {
    entries: entryFiles,
    defaultConfig: defaultConfigFilePath,
    targets: {
        browser: {
            distDir: outDir,
            context: "browser",
            isLibrary: false,
            outputFormat: "global",
            publicUrl: "./",
            // scopeHoist: true, // <-- toggling this to true does nothing
        },
    },
    disableCache: true,
    mode: process.env.NODE_ENV,
    shouldPatchConsole: false,
    detailedReport: false,
    defaultTargetOptions: {       // defaultTargetOptions needs to be present     
        shouldScopeHoist: true,   // along with shouldScopeHoist: true in order
    },                            // for scope hoisting to work
}

And once scope hoisting is working, that output from above now looks like I would expect:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            (function() {
                console.log('hello world');
            })();

        </script>
    </body>
</html>

So it seems to me there is a bug present here?

danofpaco commented 3 years ago

Ok, to complicate things further, it seems additionally dependent on what the mode is.

I've updated my simple example to look like the following:

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Test</title>
</head>
<body>
    <script>
        function sayHi () {
            console.log('hello world!')
        }
        sayHi();
    </script>
</body>
</html>

I'm seeing the following 4 types of results:

Result Type A: no scope hoisting:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            // modules are defined as an array
            // [ module function, map of requires ]
            //
            // map of requires is short require name -> numeric require
            //
            // anything defined in a previous bundle is accessed via the
            // orig method which is the require for previous bundles

            (function(modules, entry, mainEntry, parcelRequireName, globalName) {
                /* eslint-disable no-undef */
                var globalObject =
                    typeof globalThis !== 'undefined' ?
                    globalThis :
                    typeof self !== 'undefined' ?
                    self :
                    typeof window !== 'undefined' ?
                    window :
                    typeof global !== 'undefined' ?
                    global :
                    {};
                /* eslint-enable no-undef */

                // Save the require from previous bundle to this closure if any
                var previousRequire =
                    typeof globalObject[parcelRequireName] === 'function' &&
                    globalObject[parcelRequireName];

                var cache = previousRequire.cache || {};
                // Do not use `require` to prevent Webpack from trying to bundle this call
                var nodeRequire =
                    typeof module !== 'undefined' &&
                    typeof module.require === 'function' &&
                    module.require.bind(module);

                function newRequire(name, jumped) {
                    if (!cache[name]) {
                        if (!modules[name]) {
                            // if we cannot find the module within our internal map or
                            // cache jump to the current global require ie. the last bundle
                            // that was added to the page.
                            var currentRequire =
                                typeof globalObject[parcelRequireName] === 'function' &&
                                globalObject[parcelRequireName];
                            if (!jumped && currentRequire) {
                                return currentRequire(name, true);
                            }

                            // If there are other bundles on this page the require from the
                            // previous one is saved to 'previousRequire'. Repeat this as
                            // many times as there are bundles until the module is found or
                            // we exhaust the require chain.
                            if (previousRequire) {
                                return previousRequire(name, true);
                            }

                            // Try the node require function if it exists.
                            if (nodeRequire && typeof name === 'string') {
                                return nodeRequire(name);
                            }

                            var err = new Error("Cannot find module '" + name + "'");
                            err.code = 'MODULE_NOT_FOUND';
                            throw err;
                        }

                        localRequire.resolve = resolve;
                        localRequire.cache = {};

                        var module = (cache[name] = new newRequire.Module(name));

                        modules[name][0].call(
                            module.exports,
                            localRequire,
                            module,
                            module.exports,
                            this
                        );
                    }

                    return cache[name].exports;

                    function localRequire(x) {
                        return newRequire(localRequire.resolve(x));
                    }

                    function resolve(x) {
                        return modules[name][1][x] || x;
                    }
                }

                function Module(moduleName) {
                    this.id = moduleName;
                    this.bundle = newRequire;
                    this.exports = {};
                }

                newRequire.isParcelRequire = true;
                newRequire.Module = Module;
                newRequire.modules = modules;
                newRequire.cache = cache;
                newRequire.parent = previousRequire;
                newRequire.register = function(id, exports) {
                    modules[id] = [
                        function(require, module) {
                            module.exports = exports;
                        },
                        {},
                    ];
                };

                Object.defineProperty(newRequire, 'root', {
                    get: function() {
                        return globalObject[parcelRequireName];
                    },
                });

                globalObject[parcelRequireName] = newRequire;

                for (var i = 0; i < entry.length; i++) {
                    newRequire(entry[i]);
                }

                if (mainEntry) {
                    // Expose entry point to Node, AMD or browser globals
                    // Based on https://github.com/ForbesLindesay/umd/blob/master/template.js
                    var mainExports = newRequire(mainEntry);

                    // CommonJS
                    if (typeof exports === 'object' && typeof module !== 'undefined') {
                        module.exports = mainExports;

                        // RequireJS
                    } else if (typeof define === 'function' && define.amd) {
                        define(function() {
                            return mainExports;
                        });

                        // <script>
                    } else if (globalName) {
                        this[globalName] = mainExports;
                    }
                }
            })({
                "3klNt": [function(require, module, exports) {
                    function sayHi() {
                        console.log('hello world!');
                    }
                    sayHi();

                }, {}]
            }, ["3klNt"], "3klNt", "parcelRequire0bb0")

        </script>
    </body>
</html>

Result Type B: minified version of result type A:

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            ! function(e, n, o, r, t) {
                var i = "undefined" != typeof globalThis ? globalThis : "undefined" != typeof self ? self : "undefined" != typeof window ? window : "undefined" != typeof global ? global : {},
                    u = "function" == typeof i.parcelRequire0bb0 && i.parcelRequire0bb0,
                    f = u.cache || {},
                    l = "undefined" != typeof module && "function" == typeof module.require && module.require.bind(module);

                function d(n, o) {
                    if (!f[n]) {
                        if (!e[n]) {
                            var r = "function" == typeof i.parcelRequire0bb0 && i.parcelRequire0bb0;
                            if (!o && r) return r(n, !0);
                            if (u) return u(n, !0);
                            if (l && "string" == typeof n) return l(n);
                            var t = new Error("Cannot find module '" + n + "'");
                            throw t.code = "MODULE_NOT_FOUND", t
                        }
                        p.resolve = function(o) {
                            return e[n][1][o] || o
                        }, p.cache = {};
                        var c = f[n] = new d.Module(n);
                        e[n][0].call(c.exports, p, c, c.exports, this)
                    }
                    return f[n].exports;

                    function p(e) {
                        return d(p.resolve(e))
                    }
                }
                d.isParcelRequire = !0, d.Module = function(e) {
                    this.id = e, this.bundle = d, this.exports = {}
                }, d.modules = e, d.cache = f, d.parent = u, d.register = function(n, o) {
                    e[n] = [function(e, n) {
                        n.exports = o
                    }, {}]
                }, Object.defineProperty(d, "root", {
                    get: function() {
                        return i.parcelRequire0bb0
                    }
                }), i.parcelRequire0bb0 = d;
                for (var c = 0; c < n.length; c++) d(n[c]);
                var p = d("3klNt");
                "object" == typeof exports && "undefined" != typeof module ? module.exports = p : "function" == typeof define && define.amd && define((function() {
                    return p
                }))
            }({
                "3klNt": [function(e, n, o) {
                    console.log("hello world!")
                }, {}]
            }, ["3klNt"]);

        </script>
    </body>
</html>

Result Type C: Optimized, but not hoisted

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            console.log("hello world!");

        </script>
    </body>
</html>

Result Type D: Hoisted

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="UTF-8">
        <title>Test</title>
    </head>
    <body>
        <script>
            (function() {
                function $aaed6935a3c14aae11de150f882de8a8$var$sayHi() {
                    console.log('hello world!');
                }
                $aaed6935a3c14aae11de150f882de8a8$var$sayHi();
            })();

        </script>
    </body>
</html>
mode shouldScopeHoist scopeHoist Did scope hoist? Result type
"development" undefined undefined no A
"development" true undefined yes D
"development" true true yes D
"development" true false no A
"development" undefined true no A
"development" undefined false no A
"production" undefined undefined no C
"production" true undefined no C
"production" true true no C
"production" true false no B
"production" undefined true no C
"production" undefined false no B

Maybe there's just something I don't understand about how this is supposed to work, but the results seem inconsistent / unpredictable to me.

edit: goodness, it looks like the shouldOptimize setting additionally affects the results. What exactly does this setting do?

mischnic commented 3 years ago

Yes, there is something wrong here. The first boolean is whether scope hoisting actually ran. The last (non-undefined) value in each row should determine the behaviour, "!!!" means wrong behaviour.

true   mode:production , shouldScopeHoist: undefined, scopeHoist: undefined
true   mode:production , shouldScopeHoist: true     , scopeHoist: undefined
false  mode:production , shouldScopeHoist: false    , scopeHoist: undefined

true   mode:production , shouldScopeHoist: undefined, scopeHoist: ture
true   mode:production , shouldScopeHoist: true     , scopeHoist: true
false  mode:production , shouldScopeHoist: false    , scopeHoist: true // !!!

false  mode:production , shouldScopeHoist: undefined, scopeHoist: false
false  mode:production , shouldScopeHoist: true     , scopeHoist: false
false  mode:production , shouldScopeHoist: false    , scopeHoist: false

false  mode:development, shouldScopeHoist: undefined, scopeHoist: undefined
true   mode:development, shouldScopeHoist: true     , scopeHoist: undefined
false  mode:development, shouldScopeHoist: false    , scopeHoist: undefined

false  mode:development, shouldScopeHoist: undefined, scopeHoist: true // !!!
true   mode:development, shouldScopeHoist: true     , scopeHoist: true
false  mode:development, shouldScopeHoist: false    , scopeHoist: true

false  mode:development, shouldScopeHoist: undefined, scopeHoist: false
false  mode:development, shouldScopeHoist: true     , scopeHoist: false
false  mode:development, shouldScopeHoist: false    , scopeHoist: false

edit: goodness, it looks like the shouldOptimize setting additionally affects the results. What exactly does this setting do?

In this case, running Terser and htmlnano. But they are both handled the same way (incorrectly):

true   mode:production , shouldOptimize: undefined, optimize: undefined
true   mode:production , shouldOptimize: true     , optimize: undefined
false  mode:production , shouldOptimize: false    , optimize: undefined

true   mode:production , shouldOptimize: undefined, optimize: true
true   mode:production , shouldOptimize: true     , optimize: true
false  mode:production , shouldOptimize: false    , optimize: true // !!!

false  mode:production , shouldOptimize: undefined, optimize: false
false  mode:production , shouldOptimize: true     , optimize: false
false  mode:production , shouldOptimize: false    , optimize: false

false  mode:development, shouldOptimize: undefined, optimize: undefined
true   mode:development, shouldOptimize: true     , optimize: undefined
false  mode:development, shouldOptimize: false    , optimize: undefined

false  mode:development, shouldOptimize: undefined, optimize: true // !!!
true   mode:development, shouldOptimize: true     , optimize: true
false  mode:development, shouldOptimize: false    , optimize: true // !!!

false  mode:development, shouldOptimize: undefined, optimize: false
false  mode:development, shouldOptimize: true     , optimize: false
false  mode:development, shouldOptimize: false    , optimize: false
Test script ```js const Parcel = require("@parcel/core").default; const defaultConfigContents = require("@parcel/config-default"); const defaultConfigFilePath = require.resolve("@parcel/config-default"); const path = require("path"); const fs = require("fs"); async function build({ mode, scopeHoist, shouldScopeHoist, shouldOptimize, optimize, }) { await new Parcel({ entries: "index.html", defaultConfig: defaultConfigFilePath, targets: { browser: { distDir: "dist/", context: "browser", isLibrary: false, outputFormat: "global", publicUrl: "./", ...(scopeHoist != null ? { scopeHoist } : {}), ...(optimize != null ? { optimize } : {}), }, }, defaultTargetOptions: { ...(shouldScopeHoist != null ? { shouldScopeHoist } : {}), ...(shouldOptimize != null ? { shouldOptimize } : {}), }, disableCache: true, ...(mode != null ? { mode } : {}), logLevel: "warn", shouldPatchConsole: false, detailedReport: false, }).run(); return fs.readFileSync("dist/index.html", "utf8"); } (async () => { function test(contents) { return !contents.includes("previousRequire"); } for (let mode of ["production", "development"]) { for (let scopeHoist of [undefined, true, false]) { for (let shouldScopeHoist of [undefined, true, false]) { console.log( String( test( await build({ mode, scopeHoist, shouldScopeHoist }) ) ).padEnd(6), `mode:${mode.padEnd(11)}, shouldScopeHoist: ${String( shouldScopeHoist ).padEnd(9)}, scopeHoist: ${String(scopeHoist).padEnd(9)}` ); } } } })(); (async () => { function test(contents) { return contents.split("\n").length === 1; } for (let mode of ["production", "development"]) { for (let optimize of [undefined, true, false]) { for (let shouldOptimize of [undefined, true, false]) { console.log( String( test(await build({ mode, optimize, shouldOptimize })) ).padEnd(6), `mode:${mode.padEnd(11)}, shouldOptimize: ${String( shouldOptimize ).padEnd(9)}, optimize: ${String(optimize).padEnd(9)}` ); } } } })(); ```
LEW21 commented 2 years ago

This happens only when using targets as an object, not a string list. It's caused by shouldScopeHoist: this.options.defaultTargetOptions.shouldScopeHoist && descriptor.scopeHoist !== false, in core/src/requests/TargetRequest.js.