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

What is the proper way to have an isBrowser/isNode check that causes the code to only bundle one of two paths depending on platform target? #3992

Open
MicahZoltu opened this issue Nov 28, 2024 · 5 comments

Comments

@MicahZoltu
Copy link

MicahZoltu commented Nov 28, 2024

const inBrowser = (typeof window !== "undefined");
let NodeWorker;
let NodeCrypto;
if (!inBrowser) {
    NodeWorker = require("worker_threads").Worker;
    NodeCrypto = require("crypto");
}

This code will fail to bundle when --platform=browser is set because neither of those libraries are available in the browser. How can I modify the isBrowser check such that esbuild will correctly skip those lines?

@hyrious
Copy link

hyrious commented Nov 28, 2024

You can add a try-catch around the requires so that esbuild won't check inside:

const inBrowser = (typeof window !== "undefined");
let NodeWorker;
let NodeCrypto;
if (!inBrowser) {
	try {
		NodeWorker = require("worker_threads").Worker;
		NodeCrypto = require("crypto");
	} catch {}
}

@MicahZoltu
Copy link
Author

If in node, you would want an error if either of those fails. Adding a try with an empty catch would introduce a potential bug.

@MicahZoltu
Copy link
Author

Hmm, this seems to do the trick. Why is it that esbuild ignores the contents of try/catch blocks?

const inBrowser = (typeof window !== "undefined");
let NodeWorker;
let NodeCrypto;
if (!inBrowser) {
    try {
        NodeWorker = require("worker_threads").Worker;
        NodeCrypto = require("crypto");
    } catch (error) {
        // necessary so esbuild ignores this
        throw error
    }
}

@hyrious
Copy link

hyrious commented Nov 28, 2024

esbuild reports that error because it can not bundle something not exist if that thing isn't externalized. But there's a common code pattern which gracefully handles "optional dependencies" (i.e. inside a try-catch block) so esbuild also allows that to swallow the error.

esbuild/internal/ast/ast.go

Lines 115 to 125 in 9eca464

// True for the following cases:
//
// try { require('x') } catch { handle }
// try { await import('x') } catch { handle }
// try { require.resolve('x') } catch { handle }
// import('x').catch(handle)
// import('x').then(_, handle)
//
// In these cases we shouldn't generate an error if the path could not be
// resolved.
HandlesImportErrors

Other choices are that you can tell esbuild those node modules are externalized, or replaced with undefined:

  • --external:worker_threads --external:crypto (example)
  • Put this in your pakcage.json:
    "browser": { "crypto": false }
    (example)

@MicahZoltu
Copy link
Author

Is it possible to achieve the second solution (just put undefined) via CLI options, or do I have to modify the package.json to achieve that?

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