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

Add a flag that regulates whether --external preserves the import or removes it #1616

Closed
Pomax opened this issue Sep 17, 2021 · 6 comments
Closed

Comments

@Pomax
Copy link

Pomax commented Sep 17, 2021

Being able to mark things like fs or path as external is super useful, but preserving import("fs") will then cause problems at runtime because browser imports need to be resolvable URLs. As per https://esbuild.github.io/api/#external, the --external:... mechanism will prevent esbuild from bundling that target in, but will leave the require/import in place: it would be really useful to also have a secondary flag like --omit-externals=true (or a flag that acts like external, but with omission, like --external:blah --omit-external:fs) that tells esbuild to remove the import entirely, relying on the user having a globalThis binding for the namespace they were importing in place at runtime. This would make it quite a lot easier to create bundles that rely on (any dpeth of) dependencies that rely on specific Node namespaces as CLI call/npm script.

@Pomax Pomax changed the title Add a flag that regulates whether --external preserves the require/import or removes it Add a flag that regulates whether --external preserves the import or removes it Sep 17, 2021
@dvdzkwsk
Copy link
Contributor

dvdzkwsk commented Sep 18, 2021

I can't speak for what esbuild should offer out of the box, but you may be able to implement this as a plugin. Here's one I put together; does this help at all? The emitted code wraps the globalized module more than I'd like (mostly to support ESM -> CJS interop as is required in a typical TypeScript codebase), so in that sense first-class support for this feature may be helpful, but perhaps this at least gives you ideas in the interim.

esbuild.build({
    // ...
    plugins: [
        globalizeModules({
            fs: "globalThis.fs",
        }),
    ],
})

function globalizeModules(modules) {
    return {
        name: "globalize-externals",
        setup(build) {
            let ids = Object.keys(modules)
            let filter = new RegExp("^(" + ids.join("|") + ")$")
            build.onResolve({filter}, (args) => {
                return {path: args.path, namespace: "global"}
            })

            build.onLoad({filter, namespace: "global"}, (args) => {
                // you'll probably need to change the export type depending on
                // how the module is imported.
                let contents = `module.exports = ${modules[args.path]}`
                return {contents, loader: "js"}
            })
        },
    }
}

As another solution, it looks like some people were defining a global require function to resolve modules. That may be a better solution depending on your use case. Regardless, here's a self-contained script that you can run if you'd like to fiddle with it:

let cp = require("child_process")
let fs = require("fs")
let esbuild = require("esbuild")

let example = `
import * as fs from "fs"
import {readFileSync} from "fs"

// test import *
let a = fs.readFileSync("hello.txt")

// test named import
let b = readFileSync("hello.txt")

console.log({ a, b })`
fs.writeFileSync("./example.ts", example, "utf8")

function globalizeModules(modules) {
    return {
        name: "globalize-externals",
        setup(build) {
            let ids = Object.keys(modules)
            let filter = new RegExp("^(" + ids.join("|") + ")$")
            build.onResolve({filter}, (args) => {
                return {path: args.path, namespace: "global"}
            })

            build.onLoad({filter, namespace: "global"}, (args) => {
                // you'll probably need to change the export type depending on
                // how the module is imported.
                let contents = `module.exports = ${modules[args.path]}`
                return {contents, loader: "js"}
            })
        },
    }
}

esbuild
    .build({
        entryPoints: ["./example.ts"],
        outfile: "example.js",
        platform: "browser",
        bundle: true,
        logLevel: "info",
        plugins: [
            globalizeModules({
                fs: "globalThis.fs",
            }),
        ],
    })
    .then(() => {
        let test = `
        globalThis.fs = {
          readFileSync(file) {
            if (file === "a.txt") {
              return "file a"
            } else {
              return "file b"
            }
          }
        }
        require("./example.js")`
        cp.execSync(`node -e '${test}'`, {stdio: "inherit"})
    })

@Pomax
Copy link
Author

Pomax commented Sep 18, 2021

That's literally what I am hoping to avoid: I extensively use esbuild as a pure command line utility, and being able to tell --external that the import should be removed, instead of left in, would make a huge difference for that use-case.

@dvdzkwsk
Copy link
Contributor

dvdzkwsk commented Sep 18, 2021

Sure, that makes sense. I was just trying to be helpful about working around it in the interim. I'll defer to evanw regarding it being supported as a first-class option.

@evanw
Copy link
Owner

evanw commented Sep 21, 2021

I'm not sure this should be built into esbuild itself because "omission" isn't straightforward or standardized. There are various ways you might want to link these external dependencies up or replace them with a no-op object, so this seems more appropriate for a plugin to me.

@Pomax
Copy link
Author

Pomax commented Sep 21, 2021

Sure, but none of those ways would be esbuild's job I would think? How the codebase gets a reference to the now missing globals would be entirely up to the user, but being able to tell esbuild to omit instead of preserve would give them that freedom, instead of forcing folks into moving away from plain CLI invocation into "and now we need separate config files, and plugin dependencies, and basically a build system for the build system".

@evanw
Copy link
Owner

evanw commented Jul 13, 2023

I'm closing this issue because I think this is appropriate for a plugin.

@evanw evanw closed this as not planned Won't fix, can't repro, duplicate, stale Jul 13, 2023
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

3 participants