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

is.defined #52

Closed
rarkins opened this issue May 25, 2018 · 13 comments
Closed

is.defined #52

rarkins opened this issue May 25, 2018 · 13 comments

Comments

@rarkins
Copy link

rarkins commented May 25, 2018

Would you support a PR for is.defined? Clearer logic in downstream code than !is.undefined

@sindresorhus
Copy link
Owner

What would be the expected behavior for is.defined(null)?

@rarkins
Copy link
Author

rarkins commented May 25, 2018

To be true. Essentially same as expect(foo).toBeDefined() when testing

@sindresorhus
Copy link
Owner

The is.undefined() method is clear, since JS has the undefined primitive, but is.defined() could in my mind both mean that it's defined, as in not undefined, but also defined, as in not null or undefined. So I don't really find it clearer than !is.undefined() which negates the name of the primitive.

@ltetzlaff @SamVerschueren @brandon93s Thoughts?

@rarkins
Copy link
Author

rarkins commented May 25, 2018

Thanks for considering it. I agree that there’s definitely some ambiguity, but on the other hand:

  • there is also ambiguity in other functions in this package (eg you probably need to read the docs to know what isArrayLike means)
  • using “defined” to mean “!undefined” has some precedent as I mentioned, with expect

I found that the “double negative” of “not undefined” still makes me pause and re-read source code so was hoping to streamline it.

@brandon93s
Copy link
Collaborator

I think it'd be too easy to accidentally misuse an is.defined method regardless of which implementation we choose. Being "defined" doesn't seem to have a concrete definition - at least not amongst the JS community.

falsey truthy nullOrUndefinrd already cover the other use cases, so this would just be a negated version of an existing check which in general I'd like to avoid to minimize the API surface.

"Array-like" isn't the most well known term, but it does have a concrete definition in the community. It's used in the slice documents, for example.

@brandon93s
Copy link
Collaborator

If we're seeking readability, how about a not modifier?

is.not.undefined
is.not.null
is.not.arrayLike

Etc...

@rarkins
Copy link
Author

rarkins commented May 26, 2018

@brandon93s I like the idea of .not, if it's agreed that defined is too ambiguous.

@Hoishin
Copy link

Hoishin commented May 26, 2018

is.defined() could in my mind both mean that it's defined, as in not undefined, but also defined, as in not null or undefined.

Also note that undefined is a defined variable. So here is another ambiguity where is.defined() always returns true unless the given variable does not exist

@gioragutt
Copy link
Collaborator

@brandon93s this seems like the cleanest solution. I support it. At the same time, it is an answer to all use cases where you need to check a negative of an existing predicate.

@ltetzlaff
Copy link
Contributor

I'd probably not use it myself and just go with the good ol' !is.nullOrUndefined() or so but I see the point of the not function for some people I guess, at least given the nature of this repo.

Btw ArrayLike is also defined in lib.d.ts as

interface ArrayLike<T> {
    readonly length: number;
    readonly [n: number]: T;
}

and used pretty widely when "softly" referring to Enumerables but not Iterables or the like.

@sindresorhus
Copy link
Owner

I'm going to pass on this as it's too ambiguous.

As for is.not.string, I'm not sure it makes sense, since you can just use !is.string. If someone feels strongly for is.not, convince me otherwise in a new issue ;)

@bartzy
Copy link

bartzy commented Jul 12, 2022

@sindresorhus How do you assert that a value is defined? ! won't work for assertions, I think?

@sindresorhus
Copy link
Owner

@bartzy Assertions did not exist when this issue was discussed. It's generally better to open a new issue than to comment on a very old closed issue.

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

No branches or pull requests

7 participants