Skip to content

WIP: npm deps ci improvements#7074

Draft
p2edwards wants to merge 8 commits into
mainfrom
phil/npm-deps-ci-improvements
Draft

WIP: npm deps ci improvements#7074
p2edwards wants to merge 8 commits into
mainfrom
phil/npm-deps-ci-improvements

Conversation

@p2edwards

@p2edwards p2edwards commented May 21, 2026

Copy link
Copy Markdown
Contributor

Pushing some WIP commits. Not ready for review yet. —phil

Working on:

  • Batch update of npm deps
  • Whitelist dependency lifecycle scripts rather than running them indiscriminately
  • (Probably) Upgrade Node
  • (Probably) Configuring an alternate package manager with other hardened security options. If switching:
    • Update CI, Dockerfiles, and other helper scripts
    • Update documentation
    • Present a compatibility window and upgrade path (e.g., a shim or a window of time where the old commands still work)

🗒️ Checklist

  1. run linter locally
  2. update developer docs (API, README, inline, etc.), if any
  3. for user-facing doc changes create a Zulip thread at #Support Docs Updates, if any
  4. draft PR with a title <type>(<scope>)<!>: <title> DEV-1234
  5. assign yourself, tag PR: at least Front end and/or Back end or workflow
  6. fill in the template below and delete template comments
  7. review thyself: read the diff and repro the preview as written
  8. open PR & confirm that CI passes & request reviewers, if needed
  9. delete this section before merging

📣 Summary

TODO

📖 Description

TODO

👷 Description for instance maintainers

TODO

💭 Notes

TODO

👀 Preview steps

  1. ℹ️ have an account and a project
  2. do this
  3. do that
  4. 🔴 [on main] notice that this isn't anywhere
  5. 🟢 [on PR] notice that this is here
  6. do that another thing
  7. 🟢 notice that this changed like that

p2edwards added 5 commits May 21, 2026 13:08
Include repository and os arch, too. Makes debugging quicker in the
future by ruling out problem sources.
As each dependabot PR appears, every commit should be a brand new
node_modules state. Caching isn't useful until there are other
commits that would reuse it.

So during the speculative stage of dependabot PR's, we may as well save
our storage capacity. They'll appear in the cache after the `main` CI
runs.
@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This WIP PR performs a batch npm dependency upgrade alongside several CI hardening changes: a Dependabot cooldown period, a reworked cache key (now including runner.arch and skipping cache for Dependabot PRs), and the introduction of an aube.allowBuilds allowlist to gate which packages may run lifecycle scripts.

  • Dependency hygiene fixes: jest-environment-jsdom is corrected from the mismatched v29 to v30 (matching jest), coffeelint-no-implicit-returns is moved from dependencies to devDependencies, and the webpack-extract-translation-keys-plugin patch is retired after upgrading to v6.2.0 which includes the upstream fix.
  • Native-to-WASM migration: ttf2woff2 (requires native compilation) is replaced by @0x6b/ttf2woff2-wasm via an overrides alias and blocked in aube.allowBuilds, eliminating the native build step.
  • Known WIP: Several packages appear deduplicated to lower versions (e.g., fork-ts-checker-webpack-plugin ^9→^8, globals ^15→^14); the author notes these may be reverted before the PR is marked ready.

Confidence Score: 5/5

Safe to merge once the PR is marked ready — all changes are dependency version bumps and CI infrastructure updates with no production logic changes.

The changes are limited to npm dependency updates, CI workflow adjustments, and tooling configuration. No application logic is modified. The jest-environment-jsdom version mismatch is corrected, the webpack-extract-translation-keys-plugin patch is correctly retired, and the new aube.allowBuilds allowlist is an additive security improvement. The two flagged concerns are minor housekeeping issues that do not affect correctness or runtime behaviour.

No files require special attention — but the aube.allowBuilds configuration in package.json should be reviewed once aube documentation is available, as the semantics of unlisted packages are not yet clear from the PR.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CI trigger] --> B[Check actor]
    B -->|github.actor == dependabot bot| C[Skip cache step]
    B -->|Other actor| D{Check node_modules cache}
    D -->|Cache HIT| E[Run copy-fonts]
    D -->|Cache MISS| F[npm ci]
    C --> F
    E --> G[Erase node_modules/.cache]
    F --> G
    G --> H[TypeScript check]
    H --> I[Production build]
    I --> J[ESLint check]
    J --> K[Unit tests]
    K --> L[Playwright / Storybook tests]
    L --> M[Erase node_modules/.cache]
    M --> N[Cache node_modules]
Loading

Reviews (3): Last reviewed commit: "Upgrade jest-environment-jsdom" | Re-trigger Greptile

Comment thread msw-mocks/mockServiceWorker.js
Comment thread package.json
Comment on lines 76 to +90
"rtl-detect": "^1.1.2",
"select2": "^3.5.2-browserify",
"use-immer": "^0.10.0",
"wretch": "^2.9.0",
"wretch": "^2.11.1",
"zxcvbn": "^4.4.2"
},
"devDependencies": {
"@babel/preset-env": "^7.22.5",
"@0x6b/ttf2woff2-wasm": "^0.11.1",
"@babel/preset-env": "^7.29.5",
"@biomejs/biome": "^1.9.4",
"@eslint/compat": "^1.2.4",
"@eslint/compat": "^1.4.1",
"@eslint/eslintrc": "^3.3.5",
"@eslint/js": "^9.17.0",
"@eslint/js": "^9.39.4",
"@faker-js/faker": "^9.9.0",
"@jest/create-cache-key-function": "^30.3.0",
"@pmmmwh/react-refresh-webpack-plugin": "^0.5.10",
"@storybook/addon-a11y": "^9.0.0-beta.11",
"@storybook/addon-docs": "^9.0.17",
"@storybook/addon-links": "^9.1.3",
"@jest/create-cache-key-function": "^30.4.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Several packages downgraded, not upgraded

wait-on went from ^9.0.5^7.2.0 (two major versions back), globals from ^15.14.0^14.0.0, fork-ts-checker-webpack-plugin from ^9.1.0^8.0.0, and style-loader from ^4.0.0^3.3.4. Intentional downgrades are sometimes necessary to resolve compatibility issues, but without any notes in the PR it is hard to know what drove each one. Future contributors could re-raise them during a routine bump and hit the same problem again.

@p2edwards p2edwards May 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought that was odd, too. I believe that happened after I ran a dedupe command.

For example, I think fork-ts-checker-webpack-plugin was unified with the storybook subdependency:

aube why fork-ts-checker-webpack-plugin
@kobotoolbox/kpi@2.0.0 /home/phil/k/kpi

devDependencies:
@storybook/react-webpack5 9.1.20
└── @storybook/builder-webpack5 9.1.20
    └── fork-ts-checker-webpack-plugin 8.0.0
fork-ts-checker-webpack-plugin 8.0.0

I'll probably revert the dedupe step before marking this for review.

Upgrading our top-level dependencies is more important to me than unifying in these cases, although I could see a case for overriding the subdependencies if they're compatible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant