Have you tested this change on other bundlers? I'm not familiar with the current state of bundlers in the community. Is esbuild a popular one?

Have you tested this change on other bundlers?

I've tested:

• esbuild (37.5K stars, 31M weekly downloads)
• rollup (24.9K stars, 22.3M weekly downloads)
• @vercel/ncc (9K stars, 0.4M weekly downloads)

For esbuild, details are in the issue #1160

rollup

With rollup it doesn't work ouf of the box, but you can use a commonjs plugin and tweak its options so that these imports are treated dynamically (dynamicRequireTargets). This has minor security implications (Note: In extreme cases, this feature may result in some paths being rendered as absolute in the final bundle). This change doesn't affect rollup.

// rollup.config.js
var commonjs = require('@rollup/plugin-commonjs');
var nodeResolve = require('@rollup/plugin-node-resolve').nodeResolve;

module.exports = {
  input: 'index.js',
  output: {
    file: 'out.js',
    format: 'cjs',
  },
  plugins: [commonjs({
    dynamicRequireTargets: [
      'node_modules/shelljs/src/*.js',
    ],
  }), nodeResolve()],
};

// index.js
var shelljs = require('shelljs');

console.log(shelljs.pwd().stdout && '\n OK');

// bash
//$ npx rollup -c rollup.config.js

ncc

With ncc it works out of the box because ncc assumes the require could be called without the .js extension as well. (for our use-case, ncc also creates a separate exec-child.js file which isn't suitable for us). This change doesn't affect ncc.

npx ncc build index.js -o out
node out/index.js

Hello @nfischer , is there anything I can do to help make this change go live?

Would also love to get this in.shelljs is very useful for github actions

I am also highly interested in this, shelljs would be even more useful if it could be bundled! @nfischer
Currently also facing the ./src/cat issue which is so often reported in the issues of this project.

I tried the fix proposed here, but it did not work in my setup with esbuild. However, just manually writing the require statements works for me (even though it is not so elegant):

AlexanderLill#1

I have the same issue. It's very common to use esbuild to bundle VSCode extensions. Is there any plan to merge it? Thanks!

I'm willing to revisit this, however I need someone to show me how to test this change to make sure it actually works. I also see #1119 was raised for a similar issue, so I'm not sure which is the better approach.

Hello @nfischer , thanks for answering!

I'm not sure which is the better approach.

As a general rule of thumb, I say dynamic imports is something to avoid because that will always mess with build-time tree shaking. Bundlers can cope with this but this relies on blindly putting all the file content in the bundle.

TBH moving to static imports would be the preferred approach if it was my project, but I also think my change is fine if you want to keep the dynamic import.

however I need someone to show me how to test this change to make sure it actually works

For testing, I've put there a reproducer for ESBuild, do you want a reproducer for other bundlers?

Closing this in favor of #1119 and #1194