Skip to content

Effect: check if option parameter is an object. Fixed #8670 - null reference when using effects #783

Closed
wants to merge 1 commit into from

3 participants

@maciejmrozinski

No description provided.

@mikesherov
jQuery Foundation member

Thanks for contributing! Can you add a unit test for this as well?

@maciejmrozinski

I've never done that before:/ Can You guide me, tell me where to start, how unit test should looks like? What should i check within it? Thanks for any help.

@gnarf
jQuery Foundation member

options == null is better than adding the call to isPlainObject here.

@gnarf gnarf added a commit that closed this pull request Oct 21, 2012
@gnarf gnarf Effects: Allow 'null' for options - Fixes #8670 - null reference when…
… using effects - Closes gh-783
8b76684
@gnarf gnarf closed this in 8b76684 Oct 21, 2012
@gnarf
jQuery Foundation member
gnarf commented Oct 21, 2012

Thanks for forcing my hand on this one, I was planning on the == null check anyway.

@gnarf
jQuery Foundation member
gnarf commented Oct 21, 2012

P.S. my commit also included a unit test so you can check out how you go about writing one of those.

@maciejmrozinski

Thanks for this unit test. I will learn to write those for better contributing :) Please correct me if i am wrong. Is options == null the same as options == undefined?

@gnarf
jQuery Foundation member
gnarf commented Oct 25, 2012

options == null will only be true for null and undefined - It is the only case we use == inside jQuery code for exactly this reason.

@gnarf gnarf added a commit that referenced this pull request Nov 1, 2012
@gnarf gnarf Effects: Allow 'null' for options - Fixes #8670 - null reference when…
… using effects - Closes gh-783
8280997
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.