pinia icon indicating copy to clipboard operation
pinia copied to clipboard

feat: expose providePinia to use multiple root stores

Open bodograumann opened this issue 2 years ago • 8 comments

Implements #870.

bodograumann avatar Dec 06 '21 15:12 bodograumann

Codecov Report

Merging #878 (ee7911b) into v2 (421082e) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##               v2     #878   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files           9        9           
  Lines         376      378    +2     
  Branches      100      100           
=======================================
+ Hits          375      377    +2     
  Partials        1        1           
Impacted Files Coverage Δ
packages/pinia/src/rootStore.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update eaed48a...ee7911b. Read the comment docs.

codecov[bot] avatar Dec 06 '21 15:12 codecov[bot]

Thanks! I will have to give this more thought as there might be some edge cases with setActivePinia() and how it is used across stores

posva avatar Dec 06 '21 15:12 posva

Is there some way I can help with that? If you have specific scenarios in mind, I could add tests for them.

bodograumann avatar Dec 08 '21 10:12 bodograumann

Deploy Preview for pinia-official canceled.

Name Link
Latest commit f8fd0c847cb15fba60eb4ebc892d2ffef122e9ed
Latest deploy log https://app.netlify.com/sites/pinia-official/deploys/62456c8704bd690008e83dd6

netlify[bot] avatar Feb 18 '22 22:02 netlify[bot]

Currently, there are still a few issues:

  • It shouldn't change the activePinia
  • It cannot use plugins (they require the pinia._a property but even if it was set, it would still mean plugins could only be added after calling providePlugin

Overall, I think exposing the symbol and letting the user use at their own risk might be a better idea. Otherwise, a proper solution would be a custom pinia similar to the testing one

posva avatar Feb 18 '22 23:02 posva

Codecov Report

Merging #878 (7c3066b) into v2 (43f690f) will increase coverage by 0.00%. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##               v2     #878   +/-   ##
=======================================
  Coverage   99.73%   99.74%           
=======================================
  Files           9        9           
  Lines         383      385    +2     
  Branches       97       97           
=======================================
+ Hits          382      384    +2     
  Partials        1        1           
Impacted Files Coverage Δ
packages/pinia/src/rootStore.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 43f690f...7c3066b. Read the comment docs.

codecov-commenter avatar Feb 18 '22 23:02 codecov-commenter

@bodograumann would exposing the symbol be enough?

I added a couple of tests to explain what I meant. Maybe createPinia() could detect it's a nested pinia but that would also mean it would add to the library size so it should be really minimal.

There is also a concern about devtools. Right now, they expect no more than one pinia to be created for an app.

posva avatar Mar 31 '22 08:03 posva

I have a hard time wrapping my head around the whole context right now. In the feature request some more details are layed out however. My past self thought that exposing the symbol is good enough. Also, I’m only using this functionality in the context of storybook+createTestingPinia. If it helps, I could create an example project.

bodograumann avatar Apr 01 '22 13:04 bodograumann