fix(astro): Use batched stores to avoid race conditions in auth store#4000
Merged
wobsoriano merged 3 commits intomainfrom Aug 22, 2024
Merged
Conversation
🦋 Changeset detectedLatest commit: e8b3265 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
wobsoriano
commented
Aug 22, 2024
Comment on lines
+19
to
+22
| client: undefined, | ||
| user: undefined, | ||
| session: undefined, | ||
| organization: undefined, |
Member
Author
There was a problem hiding this comment.
See my comment here. Putting the update here as they're related
wobsoriano
commented
Aug 22, 2024
| "@clerk/types": "4.14.0", | ||
| "nanoid": "5.0.7", | ||
| "nanostores": "0.10.3", | ||
| "nanostores": "0.11.2", |
Member
Author
There was a problem hiding this comment.
Version bump with bug fixes and perf improvements. No breaking change. See changelog
panteliselef
approved these changes
Aug 22, 2024
| * $authStore.subscribe((auth) => console.log(auth.userId)) | ||
| */ | ||
| export const $authStore = computed([$csrState, $initialState], (state, initialState) => { | ||
| export const $authStore = batched([$csrState, $initialState], (state, initialState) => { |
Contributor
There was a problem hiding this comment.
Does this return a read only store similar to computed ?
brkalow
pushed a commit
that referenced
this pull request
Aug 22, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR fixes an issue where the
$authStorenanostore goes back to its initial values (null/undefined) after clerk-js has been loaded. See what's happening in the screen recording below:Screen.Recording.2024-08-21.at.12.20.12.PM.mov
This is happening because
computedstores react each time any of their dependencies get updated. In our case, the$csrStoreand$initialStatestores. It resets back toundefinedeven though we have an initial value because of how thederivedStorefunction works:$initialStatecoming from SSR, we have an$authStorevalue available.isLoadedproperty of$csrStoretotrue, andderivedStatewill recalculate and give us the client state and not SSR state, which at this time is still undefined since it didn't reach the listener for resources yet. This is where we getundefinedvalues again.$authStoreagain, giving us different values.This is where the
batchedstore comes in. This store will wait until the end of a tick to update itself. This is what we want forderivedStateto give a correct output in SSR and CSR:batched.mov
All other stores should work properly as they depend on the
$authStore.Fixes ECO-162
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change