Skip to content

chore(deps): drop unused content-disposition dependency#4980

Merged
aglinxinyuan merged 3 commits intoapache:mainfrom
aglinxinyuan:chore/drop-content-disposition
May 8, 2026
Merged

chore(deps): drop unused content-disposition dependency#4980
aglinxinyuan merged 3 commits intoapache:mainfrom
aglinxinyuan:chore/drop-content-disposition

Conversation

@aglinxinyuan
Copy link
Copy Markdown
Contributor

@aglinxinyuan aglinxinyuan commented May 7, 2026

What changes were proposed in this PR?

download.service.ts carried a top-level var contentDisposition = require("content-disposition") that was never read or called. Removing the dead require lets us drop the content-disposition runtime dep and its @types/content-disposition companion, plus the now-stale bundled-license entries for the transitive packages that fell out with it (base64-js, buffer, ieee754, path-browserify, safe-buffer).

Any related issues, documentation, discussions?

Closes #4979.

How was this PR tested?

  • yarn install succeeds and rewrites yarn.lock (transitive entries fall away)
  • yarn build exits 0
  • yarn test stays at 63 / 269 parity
  • CI's check_binary_deps.py --ignore-transitive-version npm …/3rdpartylicenses.json will be the final arbiter on the LICENSE-binary cleanup

Was this PR authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.7)

`download.service.ts` had a top-level
`var contentDisposition = require("content-disposition")` that was never read
or called. Removed the dead require, then dropped both `content-disposition`
and `@types/content-disposition` from package.json (and the bundled-license
roll-up). yarn.lock loses the transitive entries for free.

Closes apache#4979.
Copilot AI review requested due to automatic review settings May 7, 2026 08:18
@github-actions github-actions Bot added dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI labels May 7, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Removes an unused content-disposition require from the frontend download service and cleans up the corresponding direct dependencies and licensing manifests to keep the frontend dependency set lean.

Changes:

  • Removed dead require("content-disposition") from download.service.ts.
  • Dropped content-disposition and @types/content-disposition from frontend/package.json.
  • Updated frontend/yarn.lock and frontend/LICENSE-binary to reflect the dependency cleanup.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File Description
frontend/src/app/dashboard/service/user/download/download.service.ts Removes unused content-disposition require.
frontend/package.json Deletes direct runtime + typings dependencies for content-disposition.
frontend/LICENSE-binary Removes content-disposition@0.5.4 entry from the bundled license manifest.
frontend/yarn.lock Regenerated lockfile reflecting dependency graph updates after removal.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aglinxinyuan aglinxinyuan requested a review from bobbai00 May 7, 2026 08:29
…nsitives

The 5 packages flagged by check_binary_deps.py (base64-js, buffer, ieee754,
path-browserify, safe-buffer) were only bundled because content-disposition
pulled them in transitively. Once content-disposition itself is gone, they
fall out of the webpack bundle and the LICENSE-binary entries are stale.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.73%. Comparing base (ad0def8) to head (14c89b5).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #4980      +/-   ##
============================================
- Coverage     42.73%   42.73%   -0.01%     
+ Complexity     2187     2186       -1     
============================================
  Files          1031     1031              
  Lines         38153    38152       -1     
  Branches       4004     4004              
============================================
- Hits          16305    16303       -2     
  Misses        20831    20831              
- Partials       1017     1018       +1     
Flag Coverage Δ *Carryforward flag
access-control-service 39.53% <ø> (ø) Carriedforward from 8748b14
agent-service 33.72% <ø> (ø) Carriedforward from 8748b14
amber 43.22% <ø> (-0.01%) ⬇️ Carriedforward from 8748b14
computing-unit-managing-service 0.00% <ø> (ø) Carriedforward from 8748b14
config-service 0.00% <ø> (ø) Carriedforward from 8748b14
file-service 32.18% <ø> (ø) Carriedforward from 8748b14
frontend 33.08% <ø> (-0.01%) ⬇️
python 88.90% <ø> (ø) Carriedforward from 8748b14
workflow-compiling-service 47.72% <ø> (ø) Carriedforward from 8748b14

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aglinxinyuan aglinxinyuan requested a review from Yicong-Huang May 8, 2026 01:53
@Yicong-Huang
Copy link
Copy Markdown
Contributor

Yicong-Huang commented May 8, 2026

@aglinxinyuan what was this used for? can we git blame to trace and report here?

@aglinxinyuan
Copy link
Copy Markdown
Contributor Author

@Yicong-Huang traced it — short version: it was wired up in March, then orphaned in October when downloads moved to the browser. The require line was missed during that cleanup.

When Commit What happened
2025-03-04 9ecd32c (#3241)Fix download operators result var contentDisposition = require("content-disposition") was added so a new saveBlobFile(response, defaultFileName) helper could parse the server's Content-Disposition header and pull filename= out of it. Used on the HttpClient blob path for single-operator result download.
2025-10-10 1317199 (#3728)switch workflow result downloads to use browser native downloads Local result download switched from HttpClient blob → standard HTML form submit, so the browser handles the download (and the filename) directly. saveBlobFile (the only contentDisposition.parse caller) was deleted; the bare require at the top of the file was left behind.

So it's been dead since Oct 10, 2025 — the package and its types just rode along until I noticed.

Relevant blame on main:

9ecd32ca6a  ali risheh   2025-03-04   var contentDisposition = require("content-disposition");

And the removed call site (added in 9ecd32c, gone in 1317199):

public saveBlobFile(response: any, defaultFileName: string): void {
  const dispositionHeader = response.headers.get("Content-Disposition");
  let fileName = defaultFileName;
  if (dispositionHeader) {
    const parsed = contentDisposition.parse(dispositionHeader);
    fileName = parsed.parameters.filename || defaultFileName;
  }
  ...
}

No other references to content-disposition / contentDisposition remain under frontend/src (grepped), so dropping the dep is safe.

Copy link
Copy Markdown
Contributor

@Yicong-Huang Yicong-Huang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok let's remove it

@aglinxinyuan aglinxinyuan enabled auto-merge (squash) May 8, 2026 06:04
@aglinxinyuan aglinxinyuan merged commit b4f3a41 into apache:main May 8, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file frontend Changes related to the frontend GUI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop unused content-disposition dependency

4 participants