microsoft/azure-pipelines-task-lib

Cannot use JS bundler because of shelljs dependency

tjosepo opened this issue · 15 comments

Please check our current Issues to see if someone already reported this https://github.com/Microsoft/azure-pipelines-task-lib/issues

Environment

azure-pipelines-task-lib version: 4.3.1

Issue Description

JavaScript tasks cannot be bundled using a library like rollup or esbuild because shelljs does not explicitely declare its dependencies: shelljs/shelljs#962

There is currently a pull-request open on shelljs to address the issue, but it hasn't moved since March shelljs/shelljs#1119

Currently, the best way I found to circumvent this is to create a bundler plugin to replace the /node_modules/shelljs/shell.js file with the fixed file at compile-time.

Expected behaviour

I can bundle a task using a popular bundler like rollup or esbuild with the target platform set to node.

Actual behaviour

The task will build but most shelljs files will be missing from the bundle.

There will be a runtime exception when trying to run the task due to missing dependencies that only resolve at runtime.

Steps to reproduce

Using esbuild:

  1. Add esbuild as a devDependency:
npm i --D esbuild
  1. Create a build.mjs file with this content:
import * as esbuild from 'esbuild'

await esbuild.build({
  entryPoints: ['./src/index.js'], // or whatever your entryfile is...
  outfile: './dist/index.js',
  bundle: true,
  platform: "node",
  target: "node16",
  allowOverwrite: true,
});
  1. Run the build:
node build.mjs
  1. Run the task:
node dist/index.js
  1. An error message should appear.

Logs

$ node dist/index.js
node:internal/modules/cjs/loader:1029
  throw err;
  ^

Error: Cannot find module './src/cat'
Require stack:
- /dist/index.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:1026:15)
    at Function.Module._load (node:internal/modules/cjs/loader:871:27)
    at Module.require (node:internal/modules/cjs/loader:1098:19)
    at require (node:internal/modules/cjs/helpers:108:18)
    at /dist/index.js:2807:7
    at Array.forEach (<anonymous>)
    at node_modules/shelljs/shell.js (/dist/index.js:2806:24)
    at __require (/dist/index.js:12:52)
    at node_modules/azure-pipelines-task-lib/task.js (/dist/index.js:7287:17)
    at __require (/dist/index.js:12:52) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/dist/index.js'
  ]
}

Hi @tjosepo thanks for reporting! We are working on more prioritized issues at the moment, but will get back to this one soon.

This issue has had no activity in 90 days. Please comment if it is not actually stale

@tjosepo could you share an example of your workaround please

@tjosepo I have specified a resolutions in my package.json since I use yarn.

  "resolutions": {
    "shelljs": "Everspace/shelljs#b364c45"
  },

But the more correct/npm way is

  "dependencies": {
    "shelljs": "Everspace/shelljs#b364c45"
  },
  "overrides": {
    "shelljs": "$shelljs"
  }

This issue has had no activity in 90 days. Please comment if it is not actually stale

not stale.

UP

Please prioritize this issue if you can. This issue is preventing us from updating to recent versions of azure-pipelines-task-lib. As mentioned in the issue description, ShellJS does not plan on fixing this issue. Therefore a workaround will be needed to continue using a bundler with azure-pipelines-task-lib

@tjosepo
I've reproduced a problem with your example.

As a temporary solution you can use esbuild API external option:

import * as esbuild from 'esbuild'

await esbuild.build({
  entryPoints: ['./src/index.js'], // or whatever your entryfile is...
  outfile: './dist/index.js',
  bundle: true,
  platform: "node",
  target: "node16",
  allowOverwrite: true,
  external: ["shelljs"] // After adding this option the error no longer occurs
});

Also, I found closed issue in esbuild repository with a solution like as a comment above

I'm closing this issue.
Feel free to reopen it if you have questions about it.

7PH commented

Hey @ivanduplenskikh , putting shelljs in external is not a solution that will work all the time, and when it does not, we have to provide shelljs in node_modules which mitigates the benefit of having a tree-shaked bundle. Since we have a size limit for VSIX files, I would argue that this is a workaround and not a solution.

Is there any plan to address this? I see there were attempts at dropping shelljs in the past, for instance.

I tried to force the Shelljs modules to load. The problem is not occurring and for the moment it works.

It was used in the dependency-check for Azure DevOps library, see commit

7PH commented

See POC repository for a workaround which:

  • Puts shelljs in externals for esbuild
  • Puts an empty node_modules/shelljs/index.js file so the require(..) doesn't fail

It will work if you don't directly use methods from azure-pipelines-task-lib which actually use shelljs functions.

Feel free to comment there if you find a better solution.

7PH commented

After further investigation, fixing shelljs itself is actually possible and is a better solution because it allows to use the bundled shelljs. I've created an esbuild plugin to be used as a workaround until the issue is fixe on shelljs side (if they decide to fix it).