Skip to content

Replace createPromise() with standarized API and syntax#102

Merged
jonkoops merged 1 commit intokeycloak:mainfrom
jonkoops:standard-promises
May 19, 2025
Merged

Replace createPromise() with standarized API and syntax#102
jonkoops merged 1 commit intokeycloak:mainfrom
jonkoops:standard-promises

Conversation

@jonkoops
Copy link
Copy Markdown
Contributor

Removes the custom internal createPromise() implementation, which is a remnant of the old custom promises that Keycloak JS used to use. This replaces all the internals with the standard Promise API, and untangles a bunch of spaghetti code caused by callbacks by replacing them with async functions and awaiting them.

Closes #88

@jonkoops jonkoops requested review from a team and Copilot May 18, 2025 16:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the custom createPromise() implementation with standardized Promise API and simplifies asynchronous flow by using async/await.

  • Corrected error message text in tests to improve clarity and consistency.
  • Added test cases to assert correct behavior for token refresh including the scenario with login iframe disabled.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
test/tests/user-profile.spec.ts Updated error message text for loadUserProfile rejection
test/tests/token.spec.ts Updated error message expectation for updateToken and added extra test for iframe disabled scenario
test/tests/account-url.spec.ts Updated error message text for createAccountUrl rejection
Comments suppressed due to low confidence (3)

test/tests/user-profile.spec.ts:34

  • The error message text has been updated for clarity; please verify that it matches the standardized API expectations.
await expect(executor.loadUserProfile()).rejects.toThrow('Unable to load user profile, make sure the adapter is not configured using a generic OIDC provider.')

test/tests/token.spec.ts:10

  • The test now expects a specific error message for updateToken; ensure that the new error string aligns with the updated implementation.
await expect(executor.updateToken(9999)).rejects.toThrow('Unable to update token, no refresh token available.')

test/tests/account-url.spec.ts:34

  • The error message correction improves clarity; confirm this change is consistent with similar adjustments in other parts of the test suite.
await expect(executor.createAccountUrl()).rejects.toThrow('Unable to create account URL, make sure the adapter is not configured using a generic OIDC provider.')

Copy link
Copy Markdown
Contributor

@mposolda mposolda left a comment

Choose a reason for hiding this comment

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

Approving. Nice stuff! IMO keycloak.js looks much better now without custom promises.

I assume that this does not need any changes in the documentation and any changes in the application consuming keycloak.js. Is it correct?

@jonkoops
Copy link
Copy Markdown
Contributor Author

Correct, this is just a refactor of the internals to get away from the custom promise implementation we have internally to an implementation that uses as much standard JavaScript syntax and APIs.

Copy link
Copy Markdown
Contributor

@rmartinc rmartinc left a comment

Choose a reason for hiding this comment

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

Good job @jonkoops! Nice improvement!

@jonkoops jonkoops merged commit 2c6d61d into keycloak:main May 19, 2025
4 checks passed
@jonkoops jonkoops deleted the standard-promises branch May 19, 2025 13:16
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.

Replace createPromise() with standarized promises

4 participants