Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Update: Add support for BooleanLiteralType #173

Merged
merged 1 commit into from
Oct 13, 2016

Conversation

arv
Copy link
Contributor

@arv arv commented Sep 2, 2016

Both Flow and TypeScript (2.0) support boolean literals as types.

These are mostly used with unions as in:

type Result = {
  ok: true,
  value: string,
} | {
  ok: false,
}

@jquerybot
Copy link

Thank you for your pull request. It looks like this may be your first contribution to a jQuery Foundation project, if so we need you to sign our Contributor License Agreement (CLA).

📝 Please visit http://contribute.jquery.org/CLA/ to sign.

After you signed, the PR is checked again automatically after a minute. If there's still an issue, please reply here to let us know.


If you've already signed our CLA, it's possible your git author information doesn't match your CLA signature (both your name and email have to match), for more information, check the status of your CLA check.

@arv
Copy link
Contributor Author

arv commented Sep 2, 2016

I signed the CLA yesterday...

@nzakas
Copy link
Member

nzakas commented Sep 2, 2016

I'm sorry, I don't understand. How does this relate to JSDoc syntax?

@arv
Copy link
Contributor Author

arv commented Sep 2, 2016

It is related to the type expression language that jsdoc uses. For example:

/**
 * @return {true}
 */
function alwaysTrue() {
  return true;
}

Silly example but when you combine it with @typedef and more complex types it is useful.

For some background:
https://flowtype.org/docs/dynamic-type-tests.html#tagged-unions
https://blogs.msdn.microsoft.com/typescript/2016/08/30/announcing-typescript-2-0-rc/

@nzakas
Copy link
Member

nzakas commented Sep 3, 2016

I'm sorry, I'm having a hard time understanding. Why wouldn't you use boolean there? I don't see anything in those links that talks about JSDoc supporting this.

This utility is used by a lot of people, and I'm afraid can't stray from JSDoc expectations without some good justification or evidence that JSDoc supports what you're proposing here.

@arv
Copy link
Contributor Author

arv commented Sep 3, 2016

Let me try to justify this:

@nzakas
Copy link
Member

nzakas commented Sep 7, 2016

@arv #156 explained the use case and how JSDoc supports the syntax they were proposing. In my last comment, I asked for information showing how JSDoc works with this change. This is a JSDoc parsing tool, so the justification needs to be about JSDoc, not Flow or TypeScript.

If you'd like to provide an example JSDoc comment that makes use of this change, and explain how JSDoc handles using boolean literals instead of boolean, I'll be happy to evaluate this further.

@arv
Copy link
Contributor Author

arv commented Sep 7, 2016

Ok. Here is is a less contrived example:

/**
 * @param {{ok: true, data: string} | {ok: false, error: Error}} result
 */
function handleResult(result) {
  if (result.ok) {
    print(result.data);
  } else {
    throw result.error;
  }
}

@nzakas
Copy link
Member

nzakas commented Sep 13, 2016

And does that work with JSDoc without error?

If yes, please add that code in a unit test.

Both Flow and TypeScript (2.0) support boolean literals as types.

These are mostly used with unions as in:

```js
type Result = {
  ok: true,
  value: string,
} | {
  ok: false,
}
```
@arv
Copy link
Contributor Author

arv commented Sep 13, 2016

@nzakas Thanks for getting back to me on this. I've updated the PR with a unit test that uses that complex type.

@arv
Copy link
Contributor Author

arv commented Oct 9, 2016

What is the next step here?

@nzakas
Copy link
Member

nzakas commented Oct 13, 2016

Sorry, lost track of this. It looks good. I'll merge it and cut a new release.

@nzakas nzakas merged commit e33c6bb into eslint:master Oct 13, 2016
@arv
Copy link
Contributor Author

arv commented Oct 13, 2016

Thanks @nzakas

@arv arv deleted the add-boolean-literal branch October 13, 2016 22:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants