Skip to content

Dependency cleanup. No behaviour change to any live route. - #123

Merged
evans-g-crsj merged 6 commits into
mainfrom
chore/SJIP-1603-remove-dead-dependencies
Sep 3, 2026
Merged

Dependency cleanup. No behaviour change to any live route.#123
evans-g-crsj merged 6 commits into
mainfrom
chore/SJIP-1603-remove-dead-dependencies

Conversation

@evans-g-crsj

@evans-g-crsj evans-g-crsj commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Production closure goes from 206 to 168 packages, roughly 13 MB off the image.

  • overrides block deleted — elliptic and cross-spawn were exact-pin no-ops;
    glob ^13 forced a six-major jump onto five packages that declare ^7.x. The
    image-scan findings they were aimed at are handled by the npm 11.19.1 install in
    the prod stage.
  • joi, @types/pg-format, sequelize-cli removed — zero references. Dropping
    sequelize-cli also removes js-beautify, the last consumer of glob ^10.
  • uuidv4 → uuid@^11.1.1 — one uuid library instead of two; v11 ships its own
    types, so @types/uuid leaves too.
  • node-fetch → global fetch — smartsheet.ts was the last consumer;
    persona.ts and mailchimp.ts already used the global.
  • tsconfig lib fixed — the key sat outside compilerOptions and was silently
    ignored, so lib.dom was typing fetch, Response and structuredClone in a Node
    service. Moved inside as ["es2022"] with @types/node at ^20. target stays es6
    deliberately: es2022 turns on useDefineForClassFields, which would emit the
    Sequelize models' class fields as own properties and shadow every attribute at
    runtime — invisibly, since the tests mock sequelize.
  • type packages aligned — @types/validator to devDependencies, @types/pg
    dropped, @types/jest to ^29 to match jest.
  • eslint --fix — npm run lint now exits 0.
  • presigned upload route + @aws-sdk/client-s3 deleted — GET /user/image/presigned
    called an AWS SDK v2 API on a v3 client and always 500'd. Nothing calls it: in
    include-portal-ui uploadImageToS3 has zero call sites and the <Upload> renders no
    trigger; kf-portal-ui never called it.

PROFILE_IMAGE_BUCKET is kept on purpose — legacy profile_image_key rows are still
served from that bucket, so it can't be retired until the bucket is decommissioned.

Verified per commit with npm 11.19.1 matching the prod stage: both npm ci and
npm ci --omit=dev --omit=optional succeed with the optional subtree intact. 79/79
tests, tsc clean, lint exit 0, Trivy FS 0 HIGH/CRITICAL.

@evans-g-crsj @claude
chore: SJIP-1603 drop the stale npm overrides block
8c0e463 
All three entries are residue from an attempt to clear image-scan findings that
overrides cannot reach: they reshape our tree, not the base image's bundled npm.
That surface is handled by the npm 11.19.1 install in the prod stage instead.

elliptic and cross-spawn were no-ops. jwk-to-pem already declares elliptic
^6.6.1 and eslint/execa already declare cross-spawn ^7.0.2/^7.0.3, so both
resolve to the pinned version anyway -- and being exact pins rather than floors,
they blocked future patches. glob ^13.0.0 was the only entry that changed the
tree, forcing a six-major jump onto five packages that declare ^7.x. That broke
jest quietly: glob 13's CJS build sets __esModule with no default export, so
_interopRequireDefault yields undefined and jest-runtime's
findSiblingsWithFileExtension threw inside a bare catch, losing the "Jest was
able to find" hint on a failed module resolution.

Supersedes the rationale in 415688e. CVE-2024-21538 stays fixed because
cross-spawn still resolves to 7.0.6 without the override. CVE-2025-64756 targets
the glob CLI, which does not exist in v7 -- glob 7.2.3 ships no bin -- and the
only glob with a CLI left in the tree is js-beautify's nested 10.5.0, the fixed
version.

Verified with a clean npm ci: glob 13.0.6 -> 7.2.3 is the only version that
moves, production closure unchanged at 206 packages, Trivy FS 0 HIGH/CRITICAL,
npm audit identical to a fresh resolve with the block still in place, 79/79
tests, tsc clean, lint unchanged.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj
evans-g-crsj force-pushed the chore/SJIP-1603-remove-dead-dependencies branch from 37a855c to 8c0e463 Compare September 2, 2026 18:47
evans-g-crsj and others added 5 commits September 2, 2026 15:16
@evans-g-crsj @claude
chore: SJIP-1603 remove three dead dependencies
812fbc3 
joi had zero references anywhere in the repo -- a case-insensitive sweep of src,
migrations, test, scripts and .github found only .join( calls -- and the root
package was its only declarer in the lock file.

@types/pg-format shipped types for pg-format, which is not a dependency at all;
the types package was the only thing named pg-format in the tree.

sequelize-cli had no .sequelizerc, no seeders directory, no npm script and no
mention anywhere outside package.json. It has been unused since 335c5b6
introduced it; migrations run through node-pg-migrate.

Dropping sequelize-cli also removes js-beautify, which was the last consumer of
glob ^10, so glob collapses to a single 7.2.3 instead of two majors. 48 packages
leave the tree in total (757 -> 706) and the production closure goes from 205 to
198, since joi and @types/pg-format were both in dependencies.

@aws-sdk/client-s3 floats 3.1124.0 -> 3.1125.0 as a side effect of re-resolving
from scratch.

Verified with npm 11.19.1, matching the prod stage: both npm ci and
npm ci --omit=dev --omit=optional succeed and the optional chromedriver/fsevents
subtree is intact. 79/79 tests, tsc clean, lint unchanged, Trivy FS 0
HIGH/CRITICAL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj @claude
chore: SJIP-1603 consolidate on uuid and drop uuidv4
418707b 
The repo carried two uuid libraries for one job. uuidv4 had a single call site
in dal/user.ts while uuid was already used in routes/userSets.ts and
dal/savedFilter.ts, and uuidv4 depends on uuid 8.3.2 anyway -- it was a wrapper
over the library we already had, and npm reports it as no longer supported.

Floored at ^11.1.1 rather than ^11: uuid ships its own type declarations from v9
onward, so @types/uuid leaves the tree as well. That matters because
@types/uuid was only present as a dependency of uuidv4, so removing the wrapper
without moving to v11 would have broken the build with TS2307 at both uuid
import sites. v11 rather than the current major because npm's own deprecation
notice recommends v11 for CommonJS codebases, and this project compiles to
commonjs.

Only the import changes; the v4 call at dal/user.ts:299 is unaffected, and the
import keeps its position under simple-import-sort.

This does not clear CVE-2026-41907. sequelize declares uuid ^8.3.2 itself, so a
nested 8.3.2 copy remains in the production closure and Trivy still reports it.
It is unreachable -- the flaw is a missing buffer bounds check in v3/v5/v6 when
buf is provided, and sequelize only calls v1 and v4, never with a buffer -- and
sequelize 6.37.8 is the latest 6.x and still pins ^8.3.2, so there is no fix
short of sequelize 7.

Verified with npm 11.19.1, matching the prod stage: both npm ci and
npm ci --omit=dev --omit=optional succeed with the optional subtree intact.
79/79 tests, tsc clean with no @types/uuid installed, lint unchanged, Trivy FS
0 HIGH/CRITICAL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj @claude
chore: SJIP-1603 drop node-fetch and fix the type foundation under it
a5bea69 
smartsheet.ts was the last node-fetch consumer; persona.ts and mailchimp.ts
already used the global. Nothing in smartsheet.ts needed node-fetch specifically
-- it only uses fetch(url, init), response.ok, response.statusText and
response.json(). The test moves from jest.mock('node-fetch') to a global.fetch
mock, following the pattern mailchimp.test.ts already uses, with a named
mockFetch so the cast happens once instead of at all 26 call sites. The real
global Response works under jest's node environment, so the five
new Response(...) mocks are unchanged.

That made the type story worth fixing. tsconfig.json declared "lib": ["es2015"]
as a sibling of compilerOptions rather than inside it, so it was silently
ignored and TypeScript fell back to the default library for target es6 --
lib.es6.d.ts, which pulls in lib.dom.d.ts. fetch, Response and structuredClone
(savedFilter.ts) were all being typed by the DOM in a Node service. Moving the
key inside compilerOptions as ["es2022"] and taking @types/node to ^20 types
them from Node instead; lib.dom now leaves the program entirely.

target stays at es6 deliberately. es2022 or later would flip
useDefineForClassFields on, and the Sequelize models declare their attributes as
declaration-only class fields, which TypeScript then emits as real own
properties that shadow Sequelize's prototype accessors -- every attribute reads
undefined at runtime while the tests, which mock sequelize, stay green.

Node's stricter response.json() typing surfaced two spots the DOM's any had been
hiding: fetchSheet returned response.json() straight into Promise<Sheet> with no
cast, and mailchimp read .status off an untyped body. Both are now explicit
about being unvalidated boundaries. The mailchimp body is typed
{ status?: string } rather than the SubscriptionStatus enum, since Mailchimp also
returns cleaned, pending and transactional.

Also aligns the type packages: @types/validator moves to devDependencies where a
compile-time package belongs, @types/pg is dropped since nothing in src
references a pg type and node-pg-migrate supplies it anyway, and @types/jest
goes to ^29 to match jest ^29 -- which drops the stale jest-27 pretty-format,
jest-diff, jest-matcher-utils and react-is copies from the tree.

Verified with npm 11.19.1, matching the prod stage: both npm ci and
npm ci --omit=dev --omit=optional succeed with the optional subtree intact.
79/79 tests, tsc clean, build clean, no emitted class fields in any of the four
models, Trivy FS 0 HIGH/CRITICAL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj @claude
chore: SJIP-1603 apply eslint --fix so lint passes
7cf6a2f 
npm run lint reported 14 errors on main, all of them auto-fixable and all of
them formatting: dal/variant.ts had missing trailing commas, double-quoted
object keys and missing semicolons after function expressions; routes/userSets.ts
was missing the blank line between import groups; routes/variant.ts had an import
over the 160 column limit. No behaviour changes.

The fourteenth was arrow-body-style on the deprecated share(). Applying that fix
would have dropped the braces and left the commented-out original
implementation sitting between the arrow and a bare "true;", which reads as a
stray statement and means the rollback that block exists for now also has to
re-add the braces. Disabled the rule on that one line instead; the function is
unchanged. It goes away entirely once kf-portal-ui stops calling shareById.

Lint now exits 0. The 12 remaining warnings are unchanged: nine are
sanitizeInputPayload deliberately destructuring fields it discards, one is the
legacy createAndClauses kept for rollback, two are share()'s unused parameters.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj @claude
chore: SJIP-1603 delete the dead presigned-upload route and the AWS SDK
8f1fc58 
GET /user/image/presigned could never have worked. dal/user.ts called
S3Client.getSignedUrl('putObject', ...), an AWS SDK v2 API, on a v3 client from
@aws-sdk/client-s3, so every request threw a TypeError and returned 500. The
if (!S3Client) guard never fired either, because new S3({}) constructs fine even
with no region configured.

Nobody noticed because nothing calls it. In include-portal-ui uploadImageToS3 has
zero call sites, ProfileImageUpload renders an antd <Upload> with no children so
there is no trigger, and the uploadPhotoButton string is orphaned in all three
locales. kf-portal-ui never called the endpoint at all. Deleting rather than
fixing, since the feature is dead on both ends.

Dropping the route removes the last consumer of @aws-sdk/client-s3: 26 packages
leave the production closure, which goes from 194 to 168, and about 13 MB leaves
the image.

PROFILE_IMAGE_BUCKET stays. Legacy rows with a non-null profile_image_key are
still served from that bucket, so the variable cannot be retired until the bucket
itself is decommissioned; env.ts now says so. profileImageExtension stays for the
same reason -- it describes the naming of those existing objects. Reads of
profile_image_key are untouched: only the upload path goes.

The import block in routes/user.ts collapses to one line because removing the
symbol brought it under the 160 column limit, which prettier flags as an error.

Verified with npm 11.19.1, matching the prod stage: both npm ci and
npm ci --omit=dev --omit=optional succeed with the optional subtree intact.
79/79 tests, tsc clean, lint exit 0, Trivy FS 0 HIGH/CRITICAL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@evans-g-crsj evans-g-crsj changed the title chore: SJIP-1603 drop the stale npm overrides block Dependency cleanup. No behaviour change to any live route. Sep 2, 2026
@evans-g-crsj
evans-g-crsj merged commit 46a9b2d into main Sep 3, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@celinepelletier celinepelletier celinepelletier approved these changes
@chamorin chamorin Awaiting requested review from chamorin

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@evans-g-crsj @celinepelletier