rodgc / ngx-socket-io

Socket.IO module for Angular
MIT License
261 stars 89 forks source link

Broken Angular PeerDepencies in 4.2.0 #127

Closed kalik1 closed 1 month ago

kalik1 commented 2 years ago

In minor a version update, it changes the angular major version in peer dependencies. This change, breaks all the ^12.0.0 applications:

in 4.1.0 package.json

    "@angular/common": "^12.0.0",
    "@angular/core": "^12.0.0"

in 4.2.0 package.json

     "@angular/common": "^13.0.0",
     "@angular/core": "^13.0.0"

I think that if this angular version dependency is really needed, it should update also this package's major version, otherwise it should not change the peerDependencies section in package.json.

Quick fix: Set "ngx-socket-io": "~4.1.0", in package.json

sucotronic commented 2 years ago

I've been looking in other angular libraries and they normally change major version number at the same time that the angular dependency changes, even using the same major number.

Helveg commented 2 years ago

Sometimes packages lock step their major version with that of their peer dependency, but we have both socket.io and angular. I think since socket.io is i) less frequently changing major version ii) a less obvious peer iii) invisibly coupled to the server side as well, a more important target to lock step with than Angular. So ngx-socket-io 4.x matches with socket io 4.x

I'd therefore advise you to specify a stricter version constraint for ngx-socket-io, so that only patch version updates are automatically installed. We could update the README version support table to also list which angular versions are supported.

I'm not familiar with npm much, but can't we specify laxer peer dependencies? eg include @angular/common>12,<14?

kalik1 commented 2 years ago

As stated in SemVer in rule 7,

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API. [....]

In this case the functionality is not backward compatible, because it breaks the public API (in this case the Angular version)..

@Helveg i think that ngx-socket-io should respect semantic versioning.

I'm not familiar with npm much, but can't we specify laxer peer dependencies? eg include @angular/common>12,<14?

It should be possible (I'm not sure 100%, can't verify right now), but I definitely think this is the best solution....

github-actions[bot] commented 2 months ago

This issue is stale because it has been open for 30 days with no activity.

Helveg commented 2 months ago

@rodgc I now have a bit more experience with maintaining JS packages and I agree with this thread: We should drastically relax our peer dependency. A good plan of action here is:

github-actions[bot] commented 1 month ago

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] commented 1 month ago

This issue was closed because it has been inactive for 7 days since being marked as stale.