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

[0.3.x] Fix react-inertia setError bug #22

Merged
merged 3 commits into from
Jun 28, 2023

Conversation

yo-goto
Copy link
Contributor

@yo-goto yo-goto commented Jun 26, 2023

It seems that we cannot use the setError method of laravel-precognition-react-inertia in the current implementation.

Upon inspecting the internals, it appears that the setError method uses setErrors internally.
However, due to a mistake with this variable, attempting to use setError was resulting in it being interpreted as a non-existent property and couldn't be used.
I have made the necessary corrections to address this issue, so setError should now be properly usable.

@taylorotwell taylorotwell requested a review from timacdonald June 26, 2023 17:31
@taylorotwell taylorotwell marked this pull request as draft June 26, 2023 17:31
@timacdonald timacdonald changed the title Fix react-inertia setError bug [0.3.x] Fix react-inertia setError bug Jun 27, 2023
Comment on lines -114 to -115

return this
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unused.

@timacdonald timacdonald marked this pull request as ready for review June 27, 2023 06:33
Copy link
Member

@timacdonald timacdonald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR now removes all references to this.

When destructing an function in JS, if the function references "this" it's value is lost during the destructure.

const obj = {
    foo() {
        return this
    }
}

console.log(obj.foo() === obj)
// true

const { foo } = obj

console.log(foo() === obj)
// false

To allow better destructuring, I've removed all references to "this"

@taylorotwell taylorotwell marked this pull request as draft June 27, 2023 13:11
@yo-goto
Copy link
Contributor Author

yo-goto commented Jun 27, 2023

Yep. Thank you for improving!

@timacdonald timacdonald marked this pull request as ready for review June 28, 2023 02:22
@taylorotwell taylorotwell merged commit 9358771 into laravel:main Jun 28, 2023
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