Skip to content

Support for classes inspection #141

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

Merged
merged 5 commits into from
Nov 20, 2017
Merged

Conversation

tim-hutchinson
Copy link
Contributor

This PR has an implementation for #139 . I based the structure on the existing html and text methods that only work on a single instance, not an array.

Classes has a few tricky edge cases with CSS Modules. I don't use them in my own projects, so I don't have expertise on the right expected behavior, and couldn't find an official inverse function for them. With a CSS Module class that composes another class, both classes end up on the output. Right now, I'm returning both the extension and base classes back, but I don't know if this is the expected behavior. I'm also assuming that the base will always be defined in $style, but I haven't experience more exotic examples of CSS Modules (ie extending globals, etc).

@eddyerburgh
Copy link
Member

Thanks @tim-hutchinson . I'd like to wait for some more feedback before we merge, but let's keep this open, and I'll merge once we've decided.

@eddyerburgh
Copy link
Member

Would you be able to add attributes and props to this PR?

@tim-hutchinson
Copy link
Contributor Author

Sure, I'll get them added.

I noticed before there are tests for WrapperArray in both test/unit/specs/mount/WrapperArray and test/unit/specs/wrappers/wrapper-array.spec.js. Do you prefer tests in both locations or just one?

@eddyerburgh
Copy link
Member

Prefer tests in both locations

@tim-hutchinson
Copy link
Contributor Author

tim-hutchinson commented Nov 11, 2017

One question on the spec for props. If there are no props, should it return undefined or {}? The component itself actually has it undefined, but an empty object/array is the behavior on the other inspection functions.

@eddyerburgh
Copy link
Member

@tim-hutchinson can you fix the merge conflict?

@eddyerburgh eddyerburgh merged commit 12de854 into vuejs:dev Nov 20, 2017
@eddyerburgh
Copy link
Member

Thanks @tim-hutchinson 😀

I'll add deprecation notices to hasClass, hasProp, and hasAttribute, and release later this week

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.

2 participants