-
Notifications
You must be signed in to change notification settings - Fork 128
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
VSTS-268 Bundle and tree-shake extension task code #365
Conversation
98478dc
to
9701128
Compare
e5384bf
to
c8eaec7
Compare
/** | ||
* Hotfix for shelljs dependency | ||
* @see https://github.com/microsoft/azure-pipelines-task-lib/issues/942#issuecomment-1904939900 | ||
*/ | ||
const createShellJsDummyFile = gulpFile("index.js", "", { src: true }).pipe( | ||
gulp.dest(path.join(outPath, "node_modules", "shelljs")), | ||
); | ||
streams.push(createShellJsDummyFile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shelljs doesn't support being bundled through esbuild
, but is imported by the azure task lib. Without this fix, we can build but the task execution fails at runtime because the azure task lib always require shelljs
, even though we are not using any exported function which uses this dependency.
More explanation in this POC I made to reproduce the behavior, issue and workaround
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can also include the real shelljs into node_modules. How can we be sure we aren't using it at all on all systems?
I see it referenced inside the task.ts of the lib
https://github.com/microsoft/azure-pipelines-task-lib/blob/7c0de0dba61afb899068c35d88257017bd3b4736/node/task.ts#L2C8-L2C13
If you're confident it isn't used for any platform, I'm okay with stubbing it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not used, if we were using at some point, yes it would fail in QA and production (we should spot it in QA),
I considered adding shelljs but it's 1.7MB in each shipped task, so figured, since it's a workaround anyway we can save 2M * 15 tasks by adding this 0B file.
c8eaec7
to
d4779b1
Compare
11d74c8
to
e6db73b
Compare
e6db73b
to
43d4552
Compare
Quality Gate failedFailed conditions See analysis details on SonarQube Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Using esbuild to inline all dependencies (and drop
node_modules
from task folders)We need to to add a workaround for shelljs because we can not bundle it
Size reduction
Size before this PR, on master branch with V1+V4+V5
Size after this PR
(After rebase with the 2 new versions, V1+V2+V4+V5+V6):
Build time
On my machine, before
After