Skip to content

fix(clerk-js): Re-initialize Client singleton instance on Client destroy#1913

Merged
octoper merged 8 commits intomainfrom
vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked
Nov 2, 2023
Merged

fix(clerk-js): Re-initialize Client singleton instance on Client destroy#1913
octoper merged 8 commits intomainfrom
vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked

Conversation

@octoper
Copy link
Copy Markdown
Member

@octoper octoper commented Oct 18, 2023

Description

This PR will assign the Client properties to the default values on destroy.

Checklist

  • npm test runs as expected.
  • npm run build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

Packages affected

  • @clerk/clerk-js
  • @clerk/clerk-react
  • @clerk/nextjs
  • @clerk/remix
  • @clerk/types
  • @clerk/themes
  • @clerk/localizations
  • @clerk/clerk-expo
  • @clerk/backend
  • @clerk/clerk-sdk-node
  • @clerk/shared
  • @clerk/fastify
  • @clerk/chrome-extension
  • gatsby-plugin-clerk
  • build/tooling/chore

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Oct 18, 2023

🦋 Changeset detected

Latest commit: f78a0be

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@clerk/clerk-js Patch
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 6e01dd3 to 9aa3ded Compare October 18, 2023 12:22
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 9aa3ded to fa5e12e Compare October 18, 2023 12:36
@octoper octoper marked this pull request as ready for review October 18, 2023 12:44
@octoper octoper requested a review from a team as a code owner October 18, 2023 12:44
Comment thread packages/clerk-js/src/core/resources/Client.ts Outdated
Copy link
Copy Markdown
Contributor

@dimkl dimkl left a comment

Choose a reason for hiding this comment

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

🔧 Could you add a unit test to assert this?

Copy link
Copy Markdown
Contributor

@desiprisg desiprisg left a comment

Choose a reason for hiding this comment

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

💯

@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 6a662f4 to 9e2970e Compare October 19, 2023 10:09
Comment thread packages/clerk-js/src/core/clerk.test.ts Outdated
Comment thread packages/clerk-js/src/core/clerk.test.ts Outdated
@octoper octoper marked this pull request as draft October 19, 2023 10:28
@octoper octoper changed the base branch from main to main-v4 October 25, 2023 17:30
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 2ed1ff5 to cf5fad2 Compare October 25, 2023 17:30
@octoper octoper marked this pull request as ready for review October 25, 2023 17:41
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from cf5fad2 to 5375d0e Compare October 31, 2023 08:04
@nikosdouvlis
Copy link
Copy Markdown
Member

@octoper Can you please rebase on top of main so the new CICD changes are reflected on this PR?
Also, shall we include at least one e2e test that covers the changes made here?

@LekoArts LekoArts changed the base branch from main-v4 to main October 31, 2023 10:40
@LekoArts LekoArts requested a review from a team October 31, 2023 10:40
@octoper octoper changed the base branch from main to main-v4 October 31, 2023 10:55
@octoper
Copy link
Copy Markdown
Member Author

octoper commented Oct 31, 2023

@nikosdouvlis Do you mean the main-v4 branch right? Because this is a PR for the v4 branch.

Copy link
Copy Markdown
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

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

Let's add a E2E test for this 👍

Copy link
Copy Markdown
Contributor

@desiprisg desiprisg left a comment

Choose a reason for hiding this comment

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

💯

@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from f3af7aa to cbe1981 Compare November 1, 2023 09:49
Comment thread packages/clerk-js/src/core/resources/Client.test.ts Outdated
// @ts-expect-error This is a private method that we are mocking
BaseResource._fetch = jest.fn().mockReturnValue(
Promise.resolve({
response: clientObjectJSON,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❓ is this what the DELETE /client returns? Isn't a DeletedObject type returned?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No the DELETE /client returns a response with the following format

{
    client: null,
    response: { ... } // this is a ClientObject
}

Comment thread packages/clerk-js/src/core/resources/Client.test.ts
Comment thread packages/clerk-js/src/core/test/fixtures.ts Outdated
@dimkl dimkl self-requested a review November 1, 2023 09:54
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 7e10332 to 420ed28 Compare November 1, 2023 11:09
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch 2 times, most recently from 921309c to 260f04d Compare November 2, 2023 08:23
@octoper octoper force-pushed the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch from 260f04d to f78a0be Compare November 2, 2023 10:23
@octoper octoper enabled auto-merge November 2, 2023 10:23
@octoper octoper added this pull request to the merge queue Nov 2, 2023
Merged via the queue into main with commit 93a6115 Nov 2, 2023
@octoper octoper deleted the vaggelis/sdk-525-after-sign-out-clerkclientid-still-has-a-value-when-checked branch November 2, 2023 10:40
octoper added a commit that referenced this pull request Nov 2, 2023
…roy (#1913)

* fix(clerk-js): Re-initialize Client singleton on Client destroy

* fix(clerk-js): Assign default values instead of re-initializing Client class

* chore(repo): Adds changeset

* chore(repo): Adds changeset

* chore(clerk-js): Fix typo in test

* test(clerk-js): Added test for Client singleton destroy

* test(clerk-js): Update tests for Client singleton

* test(clerk-js): Update tests for Client singleton & created new fixtures

(cherry picked from commit 93a6115)
octoper added a commit that referenced this pull request Nov 2, 2023
…roy (#1913)

* fix(clerk-js): Re-initialize Client singleton on Client destroy

* fix(clerk-js): Assign default values instead of re-initializing Client class

* chore(repo): Adds changeset

* chore(repo): Adds changeset

* chore(clerk-js): Fix typo in test

* test(clerk-js): Added test for Client singleton destroy

* test(clerk-js): Update tests for Client singleton

* test(clerk-js): Update tests for Client singleton & created new fixtures

(cherry picked from commit 93a6115)
github-merge-queue Bot pushed a commit that referenced this pull request Nov 2, 2023
…roy (#1913) (#2016)

* fix(clerk-js): Re-initialize Client singleton on Client destroy

* fix(clerk-js): Assign default values instead of re-initializing Client class

* chore(repo): Adds changeset

* chore(repo): Adds changeset

* chore(clerk-js): Fix typo in test

* test(clerk-js): Added test for Client singleton destroy

* test(clerk-js): Update tests for Client singleton

* test(clerk-js): Update tests for Client singleton & created new fixtures

(cherry picked from commit 93a6115)

Co-authored-by: Vaggelis Yfantis <vaggelisyfantis@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants