Skip to content

Conversation

@sandersn
Copy link
Member

This was missing from the literal types PR.

@sandersn
Copy link
Member Author

@mhegazy this was easier than I thought it would be, but no less annoying since I had to duplicate JSDoc type nodes for all three types.

@sandersn
Copy link
Member Author

Looks like I need to edit baselines in some unit tests in src/harness/unittests/jsDocParsing.ts

@sandersn
Copy link
Member Author

OK, unit tests should be fixed now.

@DanielRosenwasser
Copy link
Member

It'd be good to see a test where you still get a completion list for all the members on null | undefined | { foo: number }.

Also, more of a general question (and I don't actually think you should take out the functionality), but what purpose does the never type really have in Salsa?

@DanielRosenwasser
Copy link
Member

Actually, perhaps you shouldn't see the member list until you've narrowed down.

The bug that probably isn't taken care of here is that you do show the member list for something that's nullable because in Salsa we always show members from every type in a union.

@sandersn
Copy link
Member Author

1. why never et al?

Jsdoc isn't just for salsa. It's also for --allowJs. You can pre-convert files that aren't ready to be .ts yet so that you can get good information and errors in the files that you have been able to convert to .ts. For example, you could even write assertNever in .js and use it from .ts.

2. Completion members

I'll add the test. I don't think --allowJs respects --strictNullChecks, but maybe that's an effect of the show-all-union-members behaviour you're talking about.

@sandersn
Copy link
Member Author

OK, I added the fourslash test.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 22, 2016

👍

@sandersn sandersn merged commit a013759 into master Aug 22, 2016
@sandersn sandersn deleted the jsdoc-never-undefined-null-types branch August 22, 2016 20:48
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants