fix store subscription for prop in store cases

[revintec]

What is it?

• Bug

Description

the code should be self explanatory
add the missing store subscription for prop in store cases

Checklist

the fix is tested in local dev env. I haven't contributed to qwik for some time, so please notify me if this PR needs improvement, I'll change accordingly

• My code follows the developer guidelines of this project
• I performed a self-review of my own code
• I added a changeset with pnpm change
• I added new tests to cover the fix / functionality

[changeset-bot]

🦋 Changeset detected

Latest commit: 729b82e

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

This PR includes changesets to release 4 packages

Name	Type
@builder.io/qwik	Patch
eslint-plugin-qwik	Patch
@builder.io/qwik-city	Patch
create-qwik	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

[revintec]

⚠️ No Changeset found

Latest commit: a205c7c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

it seems we don't need a changeset?

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/@builder.io/qwik@7071

npm i https://pkg.pr.new/@builder.io/qwik-city@7071

npm i https://pkg.pr.new/eslint-plugin-qwik@7071

npm i https://pkg.pr.new/create-qwik@7071

commit: f642291

[github-actions]

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
qwik-docs	✅ Ready (View Log)	Visit Preview	63d8b79

[wmertens]

How does this work? To me it seems like moving the condition will cause it to subscribe less instead of more?

And a changeset would be nice, you can add one with pnpm change

[revintec]

How does this work? To me it seems like moving the condition will cause it to subscribe less instead of more?

And a changeset would be nice, you can add one with pnpm change

it's not moving the conditions around, instead it adds this.$manager$.$addSub$ call in has()
previously code like if("key" in store)... would not get rerun when the key get set/added into the store
with this fix, they correctly do now

you maybe mistaken because I didn't squash the commits, the commits can be squashed automatically if this PR is merged according to this https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests

I've created a screenshot for your convenience

[revintec]

How does this work? To me it seems like moving the condition will cause it to subscribe less instead of more?

And a changeset would be nice, you can add one with pnpm change

and I'm having trouble running pnpm change for now, I'll try again later

[wmertens]

@revintec oh you're right, I was looking at only the diff of a single commit.

Good find, thanks!

I see that the problem also happens in v2 https://qwikdev-build-v2.qwik-8nx.pages.dev/playground/#f=Q0o0xubG2BKNDrzKpbBRAq3twSbZwg3VqM4Aec8KXFDWanLhbr3YQBoeiPq2GmI22ES9JGCBYgsxxC4xJQVYJxTBmypIjR9QKQNxQkZiMbAeAltd7RXs76cHaaFkplVqKIHFlYBFI0StZi2ijYTNGKBdWAwBimI3ArmZhTvfGFoitQxJyTcokTOaa2iRawA

Do you feel up to also fixing it in the build/v2 branch and adding a test? The code is different, but it should be pretty easy to find where to apply this, and you can see other tests for it. You can use the playground code for the test.

I'll add a changeset and merge.

[wmertens]

[wmertens]

actually, I ported it in a5972ba, so no PR needed.