This repository was archived by the owner on Jun 29, 2026. It is now read-only.
Pin yarn resolutions to close 15 of 22 Medium/Low Dependabot alerts - #49
Merged
Merged
Conversation
Continues the PR #46 sweep into Medium and Low severity. Same approach: add yarn `resolutions` for transitive dev-tool packages, regen lockfile, no source changes. The 7 remaining alerts are toolchain-blocked or have no published fix and will be dismissed-with-justification post-merge: - M #243, #139 postcss (postcss-cssnext@2 incompatible with postcss 8) - M #178, #179 webpack-dev-server (same toolchain block as PR #46's #19) - M #242 uuid (uuid 14 is ESM-only; legacy `request` can't require ESM) - M #121 request (deprecated upstream, no fix forthcoming) - L #195 elliptic (advisory fixed_in=null) Verified: yarn test (21/21), yarn lint, yarn dist:all all green. dist-modules/ AND dist/index.js byte-identical vs current master — no published artifact change at all. Closes most Medium/Low on TodayTix/ttg#15195
JerryS22
marked this pull request as ready for review
May 1, 2026 10:42
There was a problem hiding this comment.
Code Review
This pull request introduces a large number of package resolutions in package.json to pin specific versions across the dependency tree, with corresponding updates to yarn.lock. Feedback highlights significant risks associated with several of these resolutions: forcing ajv to an older major version (6.x) may break build tools requiring 8.x, while major version jumps for got (v6 to v11) and tough-cookie (v2 to v4) are likely to cause runtime failures in transitive dependencies that rely on older APIs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://github.com/orgs/TodayTix/security/overview.dist-modules/anddist/index.jsare byte-identical to current master.Resolutions added
14 resolutions → 15 alerts closed (
expresscovers two).Known exceptions (will be dismissed-with-justification post-merge)
Same toolchain-block / no-fix patterns as PR #46:
postcss8.xpixremtransitively errorsERR_PACKAGE_PATH_NOT_EXPORTED. Closing requires postcss-cssnext → cssnano migration (out of scope).webpack-dev-server5.2.1uuid14.0.0requestpackage's CommonJSrequire()can't load it (ERR_REQUIRE_ESM).requestfixed_in=null— package is deprecated upstream, no fix forthcoming.ellipticfixed_in=null. Not a higher version exists than what we're already pinned to (^6.6.1).Decisions
package.json#dependencies. Only theresolutionsblock expanded. Library API surface unchanged.Verified
yarn install(clean, Node 18.20.8)yarn run testyarn run lintyarn run dist:alldist-modules/byte-diff vs masterdist/index.jsbyte-diff vs masteryarn pack+ scratch consumer<Calendar />renders cleanly against the new tarballThe byte-stability is the headline: this PR is purely lockfile-graph cleanup. Nothing reaches consumers' bundles.
Reviewer FAQ
Q: Does merging this affect any user?
No. Identical to PR #46's analysis: this only updates the repo's
masterbranch + lockfile. No npm publish, no consumer impact, and even if someone published, bothdist/index.jsanddist-modules/are byte-identical to master so consumers' bundles wouldn't change.Q: Does platform need an update?
No. Platform consumes the prebuilt
dist/index.jsfrom npm 1.5.8 (2017). All these alerts are dev-only transitives in tt-react-calendar's webpack/babel toolchain — they never reach the runtime artifact platform ships. Platform additionally has no open npm-side Critical/High Dependabot alerts (its 51 open are all Java/Gradle).Q: Why not also fix the 7 residuals?
Each of the 7 hits a toolchain modernization wall (webpack 2 → 5, babel 6 → 7, postcss-cssnext → cssnano, ESM-only deps). That work is real but separate scope — would deserve its own design + migration plan, not a security PR.
Out of scope
masteris at 1.5.6, npm latest is 1.5.8 (2017), nothing has been published in 9 years.🤖 This was posted by Claude
Workflow progress