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

Fixed undefined key with unset method of DataStore #8231

Merged
merged 6 commits into from
Apr 3, 2024

Conversation

dmyers
Copy link
Contributor

@dmyers dmyers commented Mar 29, 2024

Ran into a new exception today after upgrading my app to the latest version.

The exception is Undefined array key "computedProperties" which I was able to trace back to #8048 and that there needed to be another isset() check on the $key.

The issue I was able to replicate that this fixes is with a legacy computed property (ex: getUserProperty()) which is using an event attribute ex: [On('someEvent')] and I guess it isn't in the data store for that kind of flow.

I updated the test added in #8048 and without the change I've made would fail, but continues to pass with the change, supporting both cases.

@joshhanley
Copy link
Member

@dmyers thanks for the PR! Please don't modify the existing tests, just add new ones. What I was testing for was a different scenario. If I disable the changes from PR #8048, my test which you modified still passes which is incorrect.

Also don't make unrelated changes like removing the render methods, as we try to keep a PR focused just on the required change. You're welcome to submit a clean up PR if you feel it's necessary (but to me having the render methods are fine).

I have reverted those two changes.

@dmyers
Copy link
Contributor Author

dmyers commented Mar 29, 2024

@joshhanley Great feedback. I appreciate your help with that, sorry for the confusion. I'm new to the Livewire internals and was trying a few different approaches to getting a solid test set up for this.

@joshhanley
Copy link
Member

joshhanley commented Mar 29, 2024

@dmyers no worries! I tested your changes, your test fails without it and passes with it, so all looks good to me 🙂

@calebporzio
Copy link
Collaborator

Thanks!

@calebporzio calebporzio merged commit 4f86fc1 into livewire:main Apr 3, 2024
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