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

Restore flexibility in keycloak-js dependency #3391

Merged
merged 1 commit into from
May 3, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"gulp": "^4.0.2",
"jest": "^27.5.1",
"js-cookie": "^3.0.1",
"keycloak-js": "21.0.2",
"keycloak-js": "^21.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

Since we know 21.0.2 at least worked, should we be setting that as the minimum?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is supposed to be taking the latest build of the major version. So, yeah; I could have updated 21.0.1 to 21.0.2, but we're really currently using 21.1.0-1, and soon something later, so that seemed pointless and even misleading. (Although there are some weird semantics here because 21.1.0 gives me the broken 21.1.0 instead of the fixed 21.1.0-1; and I gave up trying to figure out why.

Copy link
Member

Choose a reason for hiding this comment

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

If the intention here is to roll this back to "what it was", then we should be using "^21.0.1".

If we want to roll forward, then it should probably be "^21.10.0" or whatever the current version is today.

On the gripping hand, "^21.0.0" is probably more than adequate, since I think we're going to pull the latest v21 regardless of what we put here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Restoring what was there, or specifying ^21.0.2, seemed pointless as we don't expect to use 21.0.x; we expect to use the latest 21.x. Oddly, npm didn't work when I tried 21.1.0; it used exactly that (broken) version, despite the ^ which I thought ought to have avoided that. So I ended up going for the "generic" 21.0.0 "base version", which works as I expect and doesn't convey quite the same intent of a specific minor version. Really, the npm semantics for both ^ and ~ seem odd and I'm not sure either is really exactly what we ought to want; but the documentation I've found is squishy enough that I'm not entirely convinced of that, it works, it's broadly used through our package.json, and I decided not to try alternatives.

"less-watch-compiler": "^1.16.3",
"patternfly": "^3.9.0",
"react": "^17.0.2",
Expand Down