-
Notifications
You must be signed in to change notification settings - Fork 499
Add undeclared dependencies to package.json #4762
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
Conversation
We use this explicitly in the following scripts: - scripts/merge-i18n-files.ts - scripts/sync-i18n-files.ts Previously it was assumed that `commander` was available in the en- vironment, which seems to have been provided by cypress.
This undeclared dependency is used by lint/src/util/theme-support.ts.
We explicitly use this in server.ts. It has always been available because it is a dependency of express, and therefore a transitive dependency of ours. Because we are using it directly in *our* code we should declare the dependency. I am using version ^1.20.3 because that is the one used by the version of Express we are using.
This used directly in src/app/core/services/server-xhr.service.ts. We have been relying on this being available by virtue of it being a transitive dependency of some other package.
This is used directly in webpack/helpers.ts. It is a transitive de- pendency of orejime but we are using it directly in our code so we should declare it.
|
@alanorth : Did you (or could you) run |
deac9f4 to
54d44e6
Compare
|
Thanks @tdonohue. I think I figured it out. I was wondering why The problem is that I have two npm versions in my PATH, one is version 10.9.1 and one is version 11.6.1. It seems the latter changed the lockfile format or at least the behavior. I have updated the PR. |
tdonohue
left a comment
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.
👍 Thanks @alanorth ! This looks good to me. I gave it a quick test and didn't run into any issues
|
Successfully created backport PR for |
Description
Add undeclared dependencies to
package.json. I used knip to identify these, see this great blog post for the inspiration. All of these are used directly by our code, but were only available in our environment by chance as they are transitive dependencies of other packages we depend on.Instructions for Reviewers
Please add a more detailed description of the changes made by your PR. At a minimum, providing a bulleted list of changes in your PR is helpful to reviewers.
List of changes in this PR:
commanderdependency topackage.jsonglobdependency topackage.jsonand update syntax in for latest versionbody-parserdependency topackage.jsonxhr2dependency topackage.jsonmd5dependency topackage.json(note: in the future this should be handled by Node's built-in crypto API)Include guidance for how to test or review your PR. This may include: steps to reproduce a bug, screenshots or description of a new feature, or reasons behind specific changes.
Make sure the site builds in dev and prod mode. Make sure all tests in CI pass.
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.