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
Update reportUnorderedProperties
util function to also work with native classes
#574
Conversation
752b642
to
fdf69e2
Compare
tests/lib/utils/ember-test.js
Outdated
export default Component.extend({ | ||
levelOfHappiness: observer("attitude", "health", () => { | ||
}), | ||
vehicle: alias("car") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the vehicle
prop here necessary for the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♀ Nope! Copy-pasta. Will update
tests/lib/utils/ember-test.js
Outdated
|
||
it("should check if it's an observer prop", () => { | ||
node = parse('observer()'); | ||
context = new FauxContext(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about splitting each of these different "types" into their one test cases? That would help make the test output more clear about where something is going wrong, if one of these cases no longer worked right.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would generally be nice to have more, smaller unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments and we can merge this soon!
tests/lib/utils/ember-test.js
Outdated
|
||
it("should check if it's an observer prop", () => { | ||
node = parse('observer()'); | ||
context = new FauxContext(` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it would generally be nice to have more, smaller unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thank you! Can you fix the conflict due to a file being renamed? Then I will merge it.
reportUnorderedProperties
to also work with native classes reportUnorderedProperties
util function to also work with native classes
2fdd68d
to
6d4ed10
Compare
@bmish Done! |
This is needed to unblock native class compatibility for the
order-in-*
rules, which we want to fix (see #417)The
reportUnorderedProperties
function relied on a bunch of utils which currently only work for "classic" classes. This PR adds functionality which also make them compatible with native classes.