-
Notifications
You must be signed in to change notification settings - Fork 18
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
build: fix path-to-regexp to older version due to node 14 requirement #475
build: fix path-to-regexp to older version due to node 14 requirement #475
Conversation
Sorry it this broke your build, but we don't support Node versions that old, so according to our published compatibility spec we did not break any contract we adhere to, and as such it should not have been a major version bump, as it did not break any Node engine versions we support for the current version of Sinon. We do try to enforce bumping the major any time we drop support for a previous Node version, though (such as dropping Node 16 -> Sinon 17 bump). This PR was referenced from our issue tracker, so that's why I noticed, and thought I would clarify our stance on what constitutes a major version for us. |
@fatso83 thanks for the insights on that. We are figuring out how to fix that on our end, but basically it seems like all of our public Node GCP SDK are going to be blocked by that. I already notified our broad JS team here about this, to avoid duplicate work around that. |
Ah, that is unfortunate, indeed. I think the underlying dependency is not that invasive, and could probably be replaced by something else (like https://github.com/marvinhagemeister/fast-path-to-regexp ?), though I haven't tried at all. If you have any resources to look into sending a patch that would fix your issue while not re-introducing the CVE that was fixed, I am of course totally open to that. EDIT: a different take we thought of many years ago we did ponder splitting up Sinon into smaller, focused builds, allowing to drop stuff you did not need, which would have allowed some kind of |
Sinon depends on the
nise
package. That package just publishedv6.0.1
which updatedpath-to-regexp
fromv6.2.1
tov8.1.0
with sinonjs/nise#226. That should have been a major version bump, as this ended up introducing a breaking change that now it depends onNode >= 16
, which is causing our CI to break.This PR fixes
path-to-regexp
to^6.2.1
like previously to make it work onNode >= 14