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

🐛 Prevents Setting from registering a dependency #4081

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

ekwoka
Copy link
Contributor

@ekwoka ekwoka commented Mar 7, 2024

Apprently, internally Vue reacitivity would register a tracking on the key if the key was checked using Reflect.has, while getOwnProperty would not.

This using getOwnProperty first to prevent tracking on own properties, then uses Reflect.get to check for class methods.

That's a bit unexpected. 😢

but now at least there is a test for this case.

After this is pushed and released (as this is a pretty critical bug) 3.13.6 should probably be yanked/deprecated on npm.

Playground for reference of :

3.13.5

3.13.6

In the first you can see that clicking the button changes the button to clicked, while the second, the effect will run again and overwrite the change from clicked.

@joshhanley
Copy link
Collaborator

@ekwoka do you know which PR or commit broke this in 3.13.6? I will forward this on to Caleb to have a look.

@ekwoka
Copy link
Contributor Author

ekwoka commented Mar 8, 2024

Sadly, it was mine, about class methods.

@joshhanley
Copy link
Collaborator

@ekwoka all good! Ok thanks for this, I will see if Caleb can have a look 🙂

@ekwoka
Copy link
Contributor Author

ekwoka commented Mar 8, 2024

There is an alternative approach, which would be using untrack to prevent tracking the accesses that are Alpine code querying the reactive objects.

That could be more explicit about the lack of tracking on everything except the Reflect.get calls.

Oh wait, vue reactivity doesn't have untrack...does it...

@calebporzio calebporzio merged commit 28d1206 into alpinejs:main Mar 8, 2024
1 check passed
@calebporzio
Copy link
Collaborator

Thanks @ekwoka

@ekwoka
Copy link
Contributor Author

ekwoka commented Mar 10, 2024

@joshhanley @calebporzio 3.13.6 is not marked deprecated on npm, I'd recommend deprecating it (https://docs.npmjs.com/cli/v10/commands/npm-deprecate) to indicate to those using or trying to use it that they should use the newer one. Would be smart to do any time a bug is introduced :)

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.

3 participants