Skip to content
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

Update dependencies, apply fixes from "upstream" #7

Merged
merged 4 commits into from
Jul 30, 2024

Conversation

RandomByte
Copy link

This PR was mainly targeted at updating npm dependencies in order to resolve deprecation warnings produced by "npm install" right now:

❯ npm i
npm warn deprecated [email protected]: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated [email protected]: Rimraf versions prior to v4 are no longer supported
npm warn deprecated [email protected]: Glob versions prior to v9 are no longer supported

One of the dependencies switched to ESM, so I had to do the same for this package in order to use imports. Hope that is fine with you. This is a breaking change for consumers that are still using CommonJS.

While checking the origin project "davewasmer/devcert" I noticed two changes that I found reasonable to apply to this fork, see below.

I also included the 0.4.8 commit in this PR since it is missing on master but the tag is there.

I only tested these changes on macOS. Thank you for taking your time to look into this!

First Commit

refactor: Update dependencies and switch to ESM

  • Update all npm dependencies to resolve install warnings regarding
    deprecated modules.
  • Add an 'engines' declaration to package.json since the current
    version of 'glob' requires Node 20 or 22
  • Change the package to ESM since dependency 'get-port' is ESM and can't
    be require'd anymore: https://github.com/sindresorhus/get-port/releases/tag/v6.0.0

Second Commit

fix(darwin): Properly detect whether nss has been installed when using brew

Basically applying davewasmer@a1815d7 and following changes made in davewasmer/devcert

I ran into this while testing on my machine.

Third Commit

fix: Switch from execSync to execFileSync to reduce risk of remote code execution

Adapted from davewasmer@e0e8ae3

guybedford and others added 3 commits April 2, 2020 10:23
* Update all npm dependencies to resolve install warnings regarding
  deprecated modules.
* Add an 'engines' declaration to package.json since the current
  version of 'glob' requires Node 20 or 22
* Change the package to ESM since dependency 'get-port' is ESM and can't
  be require'd anymore [1]

[1]: https://github.com/sindresorhus/get-port/releases/tag/v6.0.0
…g brew

Basically applying [1] and following changes made in davewasmer/devcert

[1]: davewasmer@a1815d7
Copy link
Owner

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good to me, happy to post a release.

package.json Outdated
"scripts": {
"prepublish": "tsc"
},
"repository": {
"type": "git",
"url": "git+https://github.com/guybedford/devcert.git"
},
"engines": {
"node": "^20.11.0 || >=22.0.0"
Copy link
Owner

Choose a reason for hiding this comment

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

Node.js 14+ support ES modules, so in theory this should work there I think.

Copy link
Author

Choose a reason for hiding this comment

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

The current version of glob requires Node 20, so I thought it would be reasonable to align the minimum versions with that.

I chose 20.11 instead of 20.0 since it provides almost the same APIs as 22.0. Node 21 is already out of maintenance.

But no problem to change this to 14+ if you think that would be better.

Copy link
Owner

Choose a reason for hiding this comment

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

In this case we should not be updating to this version of glob!

Copy link
Author

Choose a reason for hiding this comment

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

Got it. The same applies to the latest version of rimraf but I don't see a problem there either.

get-port v7 requires Node 16, should we downgrade to v6 as well? I don't see relevant fixes/features in the current v7 releases.

Copy link
Author

Choose a reason for hiding this comment

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

I downgraded all three dependencies and changed the engines field to what seems reasonable based on what the dependencies state.

I have to add that I didn't test this with an older Node.js version than 20, since the project we use this package in, does not support those anymore 😅

@RandomByte
Copy link
Author

Thanks, this looks good to me, happy to post a release.

Awesome, thanks!

@guybedford guybedford merged commit 02964cd into guybedford:master Jul 30, 2024
@guybedford
Copy link
Owner

I've published a 0.5.0 here.

@flovogt
Copy link

flovogt commented Jul 30, 2024

@guybedford thanks a lot for releasing a new version. Unfortunately, the release does not contain the actual code https://www.npmjs.com/package/devcert-sanscache?activeTab=code. The whole lib folder is not present at all. I guess tsc was not executed before the release was published.

@guybedford
Copy link
Owner

@flovogt thanks I've posted a 0.5.1.

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.

None yet

3 participants