Remove unused built-in imports

[BRA1L0R]

Unused built-ins (such as fs or path) imports don't get removed but instead they get minified into this: import"fs"; or import"path";

Since built-ins don't have side-effects, I think it would be a smart thing to just get rid of them.

If this bugfix/feature is not possible, is there a way to omit them?

[arcanis]

Since built-ins don't have side-effects, I think it would be a smart thing to just get rid of them.

That wouldn't work when generating bundles for Node. Even then, side effects or not, some projects would crash at runtime if things like the path imports were silently stripped.

Perhaps you could just use the plugin API to map them into noop implementations, if that's really what you intend to do?

[evanw]

Since built-ins don't have side-effects, I think it would be a smart thing to just get rid of them.

This is interesting, and makes sense to me. It should probably be built in to the --platform=node preset and it should probably be possible to specify this property for arbitrary external modules.

That wouldn't work when generating bundles for Node. Even then, side effects or not, some projects would crash at runtime if things like the path imports were silently stripped.

Why would that be the case? If you literally add import"path"; to a file (no import names), I can't think of a reason why some projects would crash at runtime if that import was removed.

[arcanis]

Why would that be the case? If you literally add import"path"; to a file (no import names), I can't think of a reason why some projects would crash at runtime if that import was removed.

My bad, I missed that this was purely about imports lacking a symbol clause 😅

[BRA1L0R]

@arcanis

Perhaps you could just use the plugin API to map them into noop implementations, if that's really what you intend to do?

Yeah that's exactly what I did and it works, i mapped every builtin to an empty function using a regexp. I still don't get why esbuild keeps built-in imports as if they had side effects...

[BRA1L0R]

For anyone wondering this is my solution written in a little plugin:

import builtin from 'builtin-modules'

const builtinList = builtin.reduce((prev, val, index) => (index > 0 ? `${prev}|${val}` : val))
const builtinRegexp = new RegExp(`^(${builtinList})\\/?(.+)?`)

const blankImportPlugin = {
  name: 'blankimport',
  setup(build) {
    build.onResolve({ filter: builtinRegexp }, (args) => ({
      path: args.path,
      namespace: 'blankimport'
    }))
    build.onLoad({ filter: builtinRegexp, namespace: 'blankimport' }, async (args) => {
      const contents = JSON.stringify(
        /**
         * Operation steps:
         * - Import the module server side so you're 100% sure it will resolve
         * NOTE: A module won't get completely imported twice unless its import url changes, so the import is cached.
         * - get the keys of the import. This will get the name of every named export present in the built-in
         * - Reduce the string array into an object with the initializer {}, each string is added as the key of the object with an empty string
         *   so named imports will work but will just import a blank object.
         * - Finally stringify the object and let esbuild handle the rest for you
         */
        Object.keys(await import(args.path)).reduce<Object>((p, c) => ({ ...p, [c]: '' }), {})
      )
      return {
        contents,
        loader: 'json'
      }
    })
  }
}

it depends on a package called builtin-modules that is basically an array of every built-in available in the node version you are using

[Aghassi]

Assuming this is the same issue I'm facing, the way Webpack handles this is through rewriting known core libraries to empty functions if you give it hints to https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/config/webpack.config.js#L784-L795

This being said, @evanw is right that it can cause browser failures (I've seen folks not know they can't us fs for example in the browser). It would be nice to have a system like what other bundlers have, but maybe with some sort of console log or something that says "you tried to use this in the browser and can't, please fix in upstream module X"

Thanks @BRA1L0R for the plugin! Looks like a nice workaround for now

EDIT:
If the ecosystem responds and updates their code, I'm also ok with this take. There are packages that once I updated them didn't throw this error so 🤷

[lxsmnsyc]

Is there an update to this? When i'm bundling with { platform: 'browser' }, it still attempts to include these built-in modules in the build output even though these modules were never used in the tree-shaked output.