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

[SDK-3547] Rename client_id to clientId #956

Merged
merged 3 commits into from
Aug 18, 2022
Merged

Conversation

ewanharris
Copy link
Contributor

@ewanharris ewanharris commented Aug 16, 2022

Changes

Renames client_id on the public API to clientId for consistency with the JS ecosystems preference for camelCase, whilst maintaining sending client_id back to Auth0 in requests.

For consistencies sake within the codebase, except for CacheEntry the cache related code was also updated to use clientId over client_id. CacheEntry was not migrated to not break existing cached data

This will require users of this package to rename the property on their side. The diff for this looks like as follows

  • Auth0Client constructor
const auth0 = new Auth0Client({
  domain: '<AUTH0_DOMAIN>',
- client_id: '<AUTH0_CLIENT_ID>'
+ clientId: '<AUTH0_CLIENT_ID>'
});
  • createAuth0Client helper
const auth0 = await createAuth0Client({
  domain: '<AUTH0_DOMAIN>',
- client_id: '<AUTH0_CLIENT_ID>'
+ clientId: '<AUTH0_CLIENT_ID>'
});

This change also applies to the buildLogoutUrl and logout methods on Auth0Client.

However, if you are implementing a custom cache the CacheEntry structure still maintains the snakecase client_id for backwards compatibility with existing cached data so there is no migration needed for custom cache implementations.

Checklist

@ewanharris ewanharris requested a review from a team as a code owner August 16, 2022 15:19
@@ -938,7 +938,7 @@ export class Auth0Client {
: await this._getTokenFromIFrame(getTokenOptions);

await this.cacheManager.set({
client_id: this.options.client_id,
clientId: this.options.clientId,
Copy link
Member

Choose a reason for hiding this comment

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

This will put clientId in our cache, while previously it was client_id, this would make old cached values return undefined for clientId and breaking it for our users. Even though with a new major we can add breaking changes, we should try and avoid breaking it for the end-user (try and avoid logging users out with our breaking changes)

I think we should be able to either:

  • As the cache is also using snake_case, keep the client_id in CacheEntry to be snake_case
  • Keep it camelCased, but ensure it gets mapped to snake_case again before putting in cache. If we would do that, I think we should also camelCase all other properties on the cacheEntry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, moved the cache back to using client_id

@frederikprijck frederikprijck enabled auto-merge (squash) August 18, 2022 09:45
@frederikprijck frederikprijck merged commit 4b614f7 into v2 Aug 18, 2022
@frederikprijck frederikprijck deleted the refactor/SDK-3547 branch August 18, 2022 09:55
@frederikprijck frederikprijck mentioned this pull request Oct 25, 2022
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.

2 participants