Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundling issue #539

ghost · 2024-01-22T19:57:58Z

Description

Background

This is a follow-up pr to fix the issue in #522.

Mark shelljs as external for esbuild bundling 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.

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) 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

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.

Testing

Ran unit tests
ran tasks locally (using node <path-to-task>) to confirm output matches expectations
Ran tasks/pipelines on hosted server and confirmed they run successfully

Checklist

I have read the README document
I have read the CONTRIBUTING document
My code follows the code style of this project
I have added tests to cover my changes
A short description of the change has been added to the changelog using the script npm run newChange

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…dled shelljs

Requires a hacky patch to fix shell.js, which doesn't support bundlers. Mitigates the issue described in aws#539 Credit to ivanduplenskikh: https://github.com/ivanduplenskikh/aws-toolkit-azure-devops/pull/1 Co-authored-by: Ivan Duplenskikh <ivanduplenskikh>

Requires a hacky patch to fix shell.js, which doesn't support bundlers. Mitigates the issue described in aws#539 Addresses aws#566 Credit to ivanduplenskikh: https://github.com/ivanduplenskikh/aws-toolkit-azure-devops/pull/1 Co-authored-by: Ivan Duplenskikh <ivanduplenskikh>

Use Node 20 using microsoft guidance: https://learn.microsoft.com/en-us/azure/devops/extend/develop/add-build-task?view=azure-devops#q-how-can-i-upgrade-my-custom-task-to-the-latest-node Keep Node 10 as a fallback. Adding support requires a hacky patch to fix shell.js, which doesn't support bundlers. Mitigates the issue described in aws#539 Addresses aws#566 Credit to ivanduplenskikh: https://github.com/ivanduplenskikh/aws-toolkit-azure-devops/pull/1 Co-authored-by: Ivan Duplenskikh <[email protected]>

Addresses aws#566. Use Node 20 using microsoft guidance: https://learn.microsoft.com/en-us/azure/devops/extend/develop/add-build-task?view=azure-devops#q-how-can-i-upgrade-my-custom-task-to-the-latest-node Keep Node 10 as a fallback. However, we are still building the node10 with esbuild to ensure backwards compatibility. In the future, we can consider building for a higher node version only. Adding support requires a hacky patch to fix shell.js, which doesn't support bundlers. Mitigates the issue described in aws#539 Credit to ivanduplenskikh: https://github.com/ivanduplenskikh/aws-toolkit-azure-devops/pull/1 Co-authored-by: Ivan Duplenskikh <[email protected]>

Addresses #566. Use Node 20 using microsoft guidance: https://learn.microsoft.com/en-us/azure/devops/extend/develop/add-build-task?view=azure-devops#q-how-can-i-upgrade-my-custom-task-to-the-latest-node Keep Node 10 as a fallback. However, we are still building the node10 with esbuild to ensure backwards compatibility. In the future, we can consider building for a higher node version only. Adding support requires a hacky patch to fix shell.js, which doesn't support bundlers. Mitigates the issue described in #539 Credit to ivanduplenskikh: https://github.com/ivanduplenskikh/aws-toolkit-azure-devops/pull/1 Co-authored-by: Ivan Duplenskikh <[email protected]>

rishibarad added 2 commits January 22, 2024 09:32

revert azure-pipelines-task-lib to previous version that properly bun…

81990a6

…dled shelljs

remove external tag for shelljs

99f86ad

ghost self-requested a review as a code owner January 22, 2024 19:57

commit package-lock

bc485c0

ghost force-pushed the rbbarad/fix-command-not-found-issue branch from 2340396 to bc485c0 Compare January 22, 2024 20:10

awschristou approved these changes Jan 22, 2024

View reviewed changes

ghost merged commit 51f51d3 into master Jan 22, 2024

ghost deleted the rbbarad/fix-command-not-found-issue branch January 22, 2024 21:14

ghost mentioned this pull request Feb 1, 2024

Buidling code on master results in error when executing any task #522

Closed

7PH mentioned this pull request Jun 4, 2024

Dynamic require() without .js extension breaks bundling (esbuild) shelljs/shelljs#1160

Closed

This was referenced Aug 1, 2024

Revert "fix: allow tasks to run with node16" #560

Merged

Does node16 need to be added "Node runner update guidance for Azure Pipelines task authors"? #548

Closed

mhvelplund mentioned this pull request Oct 28, 2024

ECRPushImage warning in Azure Devops due to Node10 dependency #566

Closed

hayemaxi mentioned this pull request Nov 25, 2024

feat(node): support node 20 in tasks #581

Merged

5 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundling issue #539

Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundling issue #539

Uh oh!

ghost commented Jan 22, 2024 •

edited by ghost

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundling issue #539

Revert azure-pipelines-task-lib to 2.12.2 to mitigate shelljs bundling issue #539

Uh oh!

Conversation

ghost commented Jan 22, 2024 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Background

Problem

Solution

Future azure-pipelines-task-lib updates

Testing

Checklist

License

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ghost commented Jan 22, 2024 •

edited by ghost

Loading

Future `azure-pipelines-task-lib` updates