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

fix(subscriptions): allow removing subscriptions inside them #990

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

LemonAppleMo
Copy link
Contributor

@LemonAppleMo LemonAppleMo commented Jan 25, 2022

I'm sorry, my English is not very good. For a brief expression, I only used $subscribe once at that time, which will lead to exceptions in the subsequent $subscribe. I try to fix it in this way.

@netlify
Copy link

netlify bot commented Jan 25, 2022

✔️ Deploy Preview for pinia-official canceled.

🔨 Explore the source changes: 7906ddf

🔍 Inspect the deploy log: https://app.netlify.com/sites/pinia-official/deploys/61f11aae542d590007e12482

@posva
Copy link
Member

posva commented Jan 25, 2022

This doesn't seem to be necessary. Do you have a boiled down repro?

@posva posva added the need repro This issue couldn't be reproduced label Jan 25, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2022

Codecov Report

Merging #990 (7906ddf) into v2 (85daefb) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##               v2     #990   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files           9        9           
  Lines         382      382           
  Branches       97       97           
=======================================
  Hits          381      381           
  Partials        1        1           
Impacted Files Coverage Δ
packages/pinia/src/subscriptions.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 85daefb...7906ddf. Read the comment docs.

@LemonAppleMo
Copy link
Contributor Author

Example: https://github.com/LemonAppleMo/pinia-subscribe

When I subscribe to the store in two components at the same time, clearing one affects the other.

@posva posva changed the title fix(subscriptions): subscribe once and use fix(subscriptions): allow removing subscriptions inside them Jan 26, 2022
@posva posva removed the need repro This issue couldn't be reproduced label Jan 26, 2022
@posva posva merged commit 465d222 into vuejs:v2 Jan 26, 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.

None yet

3 participants