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

avoid property name conflicts in tests #401

Merged
merged 1 commit into from Dec 10, 2019
Merged

avoid property name conflicts in tests #401

merged 1 commit into from Dec 10, 2019

Conversation

faustbrian
Copy link
Contributor

1️⃣ Is this something that is wanted/needed? Did you create a feature-request issue first?
Yes. Fixes #381.

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
No.

3️⃣ Does it include tests if possible? (Not a deal-breaker, just a nice-to-have)
Yes.

4️⃣ Please include a thorough description of the feature/fix and reasons why it's useful.
Prefixes the randomly generated name property with component to avoid naming conflicts and false results in tests.

5️⃣ Thanks for contributing! 🙌

@faustbrian
Copy link
Contributor Author

faustbrian commented Dec 8, 2019

@calebporzio I think it would be a good idea to start to use a convention like componentProperty for internal things of Livewire to avoid naming conflicts with components since things like ID and name are not the only common names for properties.

Potential prefixes for internal properties could be component, internal or _ to avoid naming conflicts. Even better maybe would be to use an array that holds all state that belongs to Livewire and not the part that is implemented by developers that extend the base component. That way naming conflicts would be basically impossible and all things Livewire inside the component are isolated from the things a developer adds.

@calebporzio calebporzio merged commit b58d6dc into livewire:master Dec 10, 2019
@calebporzio
Copy link
Collaborator

Thanks @faustbrian!

And yeah, you're totally right. A dedicated array, or _ prefixed props is probably the way to go. Thanks.

@faustbrian faustbrian deleted the name-prop-tests branch December 10, 2019 15:02
@lancepioch
Copy link
Contributor

@all-contributors please add @faustbrian for test

@allcontributors
Copy link
Contributor

@lancepioch

I've put up a pull request to add @faustbrian! 🎉

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.

Testing with prop $name conflict with Livewire\Testing\TestableLivewire name prop
3 participants