-
Notifications
You must be signed in to change notification settings - Fork 20.6k
Standardize "invalid input" behavior #2134
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
Comments
I'm all for this in theory, but have the queasy feeling that there are probably cases where existing code is depending on undocumented fail-silent or other quirky behavior. |
Per today's meeting, we agreed that we'd like to try out some changes and see how it goes. Let's nail down all of the places we'd like to see this change. |
I think we all agree here, but we'll need to address each change on a case-by-case basis and assess its impact on the community. Rather than making sweeping changes across the board and asking everyone to make lots of adjustments to their code, we're going to do this incrementally, in stages. The niche use cases can be done first with a few of the more common cases being introduced every major release. We already have some changes in the pipeline. For any other changes, let's have specific issues opened for each one. I'm closing this issue because we've reached a consensus and now it's too broad. |
You closed this without making more specific tickets... are we just going to independently address unsolicited pull requests removing guards? And even if the answer to the above is affirmative, I think we should include the principle—"assume function input matches a documented signature"—somewhere in the repository for future reference. Maybe CONTRIBUTING.md? |
Right. What I'd like to move away from is leaving issues open that could not be closed by a single pull request. Opening an issue for discussion is fine, but once the discussion resolves, it should be closed. We can track the broader guidelines and overarching goals in the Roadmap. |
Many jQuery collection operations are only meaningful on elements or (somewhat less commonly) DOM nodes, but the collections can logically contain anything. Such operations should always assume valid input, potentially throwing exceptions when they encounter an invalid member—especially given the ease of manually cleaning like
$collection.filter("*")
..attr( name )
,.css( name )
,.offset()
).hasClass( clazz )
).attr( name, val )
,.css( props )
,.offset( n )
,.addClass( clazz )
)Special consideration is warranted for single-member reads responding to empty collections. Current behavior usually returns
undefined
, which seems reasonable to standardize on (including changing those methods that returnnull
, like.scrollTop()
and.width()
). There are cases where this collides with real output, but such collisions seem to be sane—$el.css( unknownProperty )
,$el.prop( nonexistent )
,$hasNoData.data()
, etc.Related:
The text was updated successfully, but these errors were encountered: