X Tutup
The Wayback Machine - https://web.archive.org/web/20200529214843/https://github.com/airbnb/javascript/pull/2148
Skip to content
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

[guide]Fix Type Error issue occur because of default value of function… #2148

Open
wants to merge 1 commit into
base: master
from

Conversation

@mhjariwala
Copy link

mhjariwala commented Jan 5, 2020

In javascript style guide, According to rule 7.7, if i trying to access property key of opts by using dot notation(.) or square bracket([]) there is chances of type error.
Why?
Because, if opts receive null value when 'handleThings' function invoked then default value empty object({}) not assign to the opts argument. So, this is happen then user will get uncaught type error.
Example:

function handleThings(opts = {}) {
console.log(opts.notificationEnabled)
 }
handleThings(null)

When you run this code you will get an error called "Uncaught TypeError: Cannot read property 'notificationEnabled' of null".

@ljharb
Copy link
Collaborator

ljharb commented Jan 5, 2020

While this is accurate, this isn't often a problem in practice, and using || isn't a better alternative since it's based on truthiness and not based on nullishness.

@mhjariwala
Copy link
Author

mhjariwala commented Jan 5, 2020

While this is accurate, this isn't often a problem in practice, and using || isn't a better alternative since it's based on truthiness and not based on nullishness.

I think we should below code to check whether opts variable contain object or not.

const newOpts = typeof opts === 'object' && opts !== null ? opts : {}
@ljharb
Copy link
Collaborator

ljharb commented Jan 5, 2020

That would be most robust, but in this case the error from passing null is desired, because that’s not how the function is intended to be used. I think this example is good as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.
X Tutup