-
Notifications
You must be signed in to change notification settings - Fork 274
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
Cannot use JS bundler because of shelljs dependency #942
Comments
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 "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 |
…g issue (#539) ### Background This is a follow-up pr to fix the issue in #522. [Mark shelljs as external for esbuild bundling](#537) attempted to address this issue, but was not a successful solution for all of our tasks, as some of our tasks need the commands present in ShellJS. ### Problem This issue emerged with the bump of `azure-pipelines-task-lib` dependency in: #473. Problem detailed in the open issue on `azure-pipelines-task-lib` repo: [Cannot use JS bundler because of shelljs dependency](microsoft/azure-pipelines-task-lib#942). Essentially, the toolkit takes a dependency on `azure-pipelines-task-lib` which takes an additional dependency on ShellJS. Our toolkit uses the esbuild JS Bundler to build the extension. This fails to bundle ShellJS because it uses dynamic require statements ([problematic ShellJS code](https://github.com/shelljs/shelljs/blob/a6d1e49f180a495d83bcf67bd85782c626aae029/shell.js#L23-L26)) instead of explicit require statements, which prevents bundlers from correctly analyzing the dependencies. - In short, require('./a') works but require('./' + 'a') doesn't. [ShellJS has no plans to accommodate these explicit requires statements](shelljs/shelljs#962 (comment)) ### Solution Before making any larger scale changes, let's get `azure-pipelines-task-lib` back to its last known healthy state (working version we used for last release). This PR addresses that. ### Future `azure-pipelines-task-lib` updates Whenever we next bump the `azure-pipelines-task-lib` major version, we'll need to keep an eye on updates to microsoft/azure-pipelines-task-lib#942 for any new fixes or workarounds. Current known workarounds involve forking the ShellJS repository, making the necessary changes to ShellJS, and overriding the ShellJS dependency in `package.json` with the forked version. - [Example of forked ShellJS with hardcoded explicit `requires` statements](Everspace/shelljs@4e2ea23) - [Example of creating a bundler plugin with a fixed ShellJS file at compile time](https://github.com/tjosepo/azure-pipelines/blob/main/packages/esbuild-plugin-azure-pipelines-task-lib-fix/src/index.js)
Please prioritize this issue if you can. This issue is preventing us from updating to recent versions of |
@tjosepo 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. |
Hey @ivanduplenskikh , putting 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 |
See POC repository for a workaround which:
It will work if you don't directly use methods from Feel free to comment there if you find a better solution. |
After further investigation, fixing |
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
oresbuild
because shelljs does not explicitely declare its dependencies: shelljs/shelljs#962There 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
oresbuild
with the target platform set tonode
.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
:esbuild
as a devDependency:build.mjs
file with this content:Logs
The text was updated successfully, but these errors were encountered: