Data: do not create data cache when fetching single property #2554
Conversation
There was a problem hiding this comment.
at the top.
We have an example of something like that in the guide https://contribute.jquery.org/style-guide/js/#spacing, but strictly speaking that is not a requirement anymore
There was a problem hiding this comment.
I'm not sure what you mean. This is failing because key is missing a declaration.
There was a problem hiding this comment.
i mean you could do
for ( var key in div[ 0 ] )
There was a problem hiding this comment.
for ( var key in div[ 0 ] )I really don't like this form. :) var is function scoped and we write it outside of any blocks inside a function to not suggest it behaves otherwise.
(i.e. this should be done exactly as this PR does it)
There was a problem hiding this comment.
I really don't like this form. :)
This is why we have a code-style, i don't like a lot of things in current code base, as i'm sure we all do, but we choose to follow it.
(i.e. this should be done exactly as this PR does it)
No, test should trigger the error with minimum overhead, any other requirements are subjective
There was a problem hiding this comment.
This is why we have a code-style
We can codify this, sure, if JSCS can do it. It's just how it is in the whole code base so it's a de-facto standard anyway.
There was a problem hiding this comment.
We can codify this, sure, if JSCS can do it.
It totally can, it was like that for a while, but code-style changed and rule was removed from the preset. It is like that de-facto because code-style used to force it, but now it doesn't, so we shouldn't ask contributor to do something, that is not documented
|
I like this. |
jbedard
force-pushed
the
data-create-on-get
branch
from
e736a11 to
1998d20
Compare
|
Oops. Should be fixed now... |
|
Landed, thanks. I had to modify the assertion count as the test should invoke only one assertion. |
5 participants
This makes master like compat. The test can probably be put into compat as is.