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

Using --external:./node_modules/* results in relative import instead of bare import #1958

Closed
supachailllpay opened this issue Jan 25, 2022 · 7 comments
Labels

Comments

@supachailllpay
Copy link

From entry.js:

import React from 'react'

Run build:

esbuild entry.js --bundle --outfile=out.js --format=esm --external:"./node_modules/*"

Then out.js:

import React from "./node_modules/react/index.js";

But if I just exclude react directly:

esbuild entry.js --bundle --outfile=out.js --format=esm --external:react

Then the result will be:

import React from "react";

Or if I use the plugin below to manually exclude all packages from node_modules. The result will still be a bare import.

let makeAllPackagesExternalPlugin = {
  name: 'make-all-packages-external',
  setup(build) {
    let filter = /^[^.\/]|^\.[^.\/]|^\.\.[^\/]/ // Must not start with "/" or "./" or "../"
    build.onResolve({ filter }, args => ({ path: args.path, external: true }))
  },
}

Originally posted by @evanw in #619 (comment)

So I'm not sure if this is the intended behavior or bug?. And if this is intended, how can I do this without using a plugin?

Thank you.

@matthiasg
Copy link

I just stumbled into this as well. This causes npx to be broken though as the node_modules folder is not created in the same location after 'install'.

matthiasg added a commit to aeppic/pack that referenced this issue Jan 26, 2022
Fixed to exluding externals explicitely

Note previously used node_modules/* based on documentation but
that rewrite to relative paths which do not follow node
module resolution rules. evanw/esbuild#1958
@maximilianschmitt
Copy link

I've also encountered this issue while trying to use esbuild to bundle some TS code for an Electron application. import ejs from 'ejs' becomes require("../../node_modules/ejs/lib/ejs.js") in the resulting bundle which then creates issues when run with Electron because when bundling the application, the node_modules move relative to the entry-file.

@StarpTech
Copy link

Hi, I run into this as well. This only happens for require imports. It would be great to have a continuation on this. As a workaround, I mark all packages in node_modules as external.

@ShravanSunder
Copy link

ShravanSunder commented Jun 1, 2022

i'm also running into this with juliencrn/usehooks-ts#97
dependant modules are turned into common js

AntonioMeireles added a commit to AntonioMeireles/homebridge-vieramatic that referenced this issue Jun 11, 2022
This reverts commit c678f84.

trigger pulled too soon:
- evanw/esbuild#619 was gone
- but then come evanw/esbuild#1958
@jakebailey
Copy link

jakebailey commented Sep 19, 2022

Not sure if you've already considered it, but this would be a great breaking change to include in something like #2508.

I've personally worked around this by doing something like const moduleName = "source-map-support" and then require-ing that, since I need to enable that if it works (but there's no guarantee that any particular version of that library has the final mapped path).

EDIT: Don't do the above... Just do --external:source-map-support; doh.

@jakebailey
Copy link

jakebailey commented Oct 13, 2022

I think that this is the same bug, but one thing I've noticed is that if you set --external:./node_modules/* but are targeting the CJS format, you will end up with code like:

chai = __toESM(require("../../node_modules/chai/index.mjs"));

Which will fail to work since that's not CJS code. If the code had not been external, this would have been fine since it'd be bundled and so the MJS-ness of the file would not matter.

Setting the conditions to be require fixes things, but it'd be best to emit require("chai") as the original code wrote. (Of course you shouldn't ship this to anyone; their version of the lib may use a different path; in my case it's a test bundle so it's fine.)

@jakebailey
Copy link

After viewing some of the cross-linked PRs, I realized that I had completely glossed over the fact that you can just specify each package name as "external" directly, rather than using a wildcard. With that in mind, I've ended up going with using a plugin that enforces that "external" is fully configured:

{
    name: "no-node-modules",
    setup: (build) => {
        build.onLoad({ filter: /[\\/]node_modules[\\/]/ }, () => {
            // See: https://github.com/evanw/esbuild/issues/1958
            return {
                errors: [{ text: 'Attempted to bundle from node_modules; ensure "external" is set correctly.' }]
            };
        });
    }
},

This gives me a handy error at bundle time:

image

This is plenty for my needs.

@evanw evanw closed this as completed in 7277ffd Dec 13, 2022
AntonioMeireles added a commit to AntonioMeireles/homebridge-vieramatic that referenced this issue Dec 26, 2022
Signed-off-by: António Meireles <antonio.meireles@reformi.st>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants