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

When accessing stores outside the rendering context, store composition will get data from the wrong instance #2004

Closed
DrPhil opened this issue Feb 15, 2023 · 4 comments

Comments

@DrPhil
Copy link

DrPhil commented Feb 15, 2023

Reproduction

https://jsfiddle.net/tsz32r68/1/

Steps to reproduce the bug

  1. Open the JSFiddle.
  2. Read output in the console.

Expected behavior

The JSFiddle ends with logging the user+cart contents:

Promise.all([
    useCart().logItems(),
    useCart(instanceA).logItems(),
    useCart(instanceB).logItems()
]);

The documentation is clear that the first logItems can log anything: we are outside Vue's context and do not pass the pinia instance to the store. The second line explicitly passes instanceA so we expect it to log Alice's items. The third line explicitly passes instanceB so we expect it to log Bob's items.

All in all, we expect something like:

"Bob: bread"
"Alice: apples, oranges"
"Bob: bread"

Actual behavior

The logs for the second line mixes the data. We explicitly pass the pinia instance for Alice, so we expect Alice's name and Alice's items. We get a mix of data: Bob's name and Alice's items.

"Bob: bread"
"Bob: apples, oranges"
"Bob: bread"

Additional information

This showed up for us in SSR. When doing SSR we want to preload some data to the Pinia stores, before we even render anything. We use serverPrefetch to load the data. As pointed out in the docs pinia adds this.$pinia that we can use to provide the right instance to our stores. But the stores themselves will still get data from the wrong instance when they look for data on other stores.

In most cases this just happens to work the right way anyway for us. Only when the previous request hit an error during SSR would Pinia pick the wrong instance.

I'll can try set up a minimal SSR repo showing this behavior in serverPrefetch too, if that would be helpful for you.

posva added a commit that referenced this issue Feb 16, 2023
@posva
Copy link
Member

posva commented Feb 16, 2023

This is because you are calling an async operation in your action before calling the useStore():

actions: {
        async logItems() {
            await simulatedBackendCall();
            const user = useUser();
            console.log(`${user.name}: ${this.items.join(', ')}`)
        }
    }

You **have to move the useUser() call before the await. Otherwise, the pinia context could be incorrect:

actions: {
        async logItems() {
            const user = useUser();
            await simulatedBackendCall();
            console.log(`${user.name}: ${this.items.join(', ')}`)
        }
    }

I updated the documentation to explain this.

@posva posva closed this as completed Feb 16, 2023
@posva
Copy link
Member

posva commented Feb 16, 2023

I'll can try set up a minimal SSR repo showing this behavior in serverPrefetch too, if that would be helpful for you.

The repro you shared, without SSR, is actually way easier to understand, thank you for that 😄

@DrPhil
Copy link
Author

DrPhil commented Feb 16, 2023

I think your documentation additions are good!

By the way, our SSR issue seems to be a combination with a bug in Vue also. I think currentInstance is not set to null if there's an error in rendering and then another error is thrown in errorCaptured. I'm not entirely sure about this yet, but will send a bug report to Vue if I can create a repro about it.

Thanks again! Great documentation clarification, and thanks for being so quick about it.

@DrPhil
Copy link
Author

DrPhil commented Feb 16, 2023

Btw, here's the Vue bug I talked about if you are curious: vuejs/core#7733

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

No branches or pull requests

2 participants