Repository navigation
Methods are not new identifier definition locations #42595
Description
let x = {
/**/
};Type in a( and we'll auto-complete the property. That shouldn't happen.
{{ message }}
let x = {
/**/
};Type in a( and we'll auto-complete the property. That shouldn't happen.
Activity
andrewbranch commentedon Feb 2, 2021
addEventListeneris a global, so it is possible that you wanted to writeIt was added in #41539.
I argued that you probably don’t ever want to do this with globals, but didn’t realize that(was a commit character (IMO, way too many things are commit characters), so I left it up to dissenters, and none showed up:Edit: everything else I said was based on a misunderstanding of what “new identifier location” was supposed to do
andrewbranch commentedon Feb 2, 2021
Daniel Rosenwasser (@DanielRosenwasser) I bring this up because I think your title for this issue is too strong of an assertion. That is a new identifier location, or else we have to revert #41539, which fixes an issue that you called a bug 😅. I can definitely get behind removing globals from the list here, but if you were in a file with a ton of locals, and had the intention to write a non-contextulaly-typed object literal with bespoke member names, you could hit the same frustration that typing(gives you a bad completion.DanielRosenwasser commentedon Feb 2, 2021
There's 2 things:
isNewIdentifierLocationandisNewIdentifierDefinitionLocationfrom what I recall)The latter should permit people to write out a new name without a commit character committing to an arbitrary global or local.
DanielRosenwasser commentedon Feb 2, 2021
That "often" has more to do with a contextual type. If you have a contextual type, you probably just want to provide those completions.
DanielRosenwasser commentedon Feb 2, 2021
It sounds like there's another bug here which is that
isNewIdentifierDefinitionLocationis very poorly documented. 😅The idea is that there's 3 states I have in mind
isNewIdentifierLocation)isNewIdentifierDefinitionLocation() -> true)And I'm saying we want the third one.
[-]Methods are not new identifier locations[/-][+]Methods are not new identifier definition locations[/+]andrewbranch commentedon Feb 2, 2021
Got it, I thought you were talking about (2) but you were talking about (3).
andrewbranch commentedon Feb 2, 2021
If an object literal has a contextual type, are its members still new identifier definition locations? That function currently works off syntax alone.
Edit: actually, it pretty much Just Works—
isNewIdentifierDefinitionLocationonly runs when we collect locals/globals, which we don’t do under a contextual type. The only questionable result isHere, we offer exactly one completion,
xyz, butisNewIdentifierLocationis set tofalse. Since the contextual type is only a constraint, it really is a new identifier definition location, but I think it doesn’t matter much since there are no extraneous completions.DanielRosenwasser commentedon Feb 2, 2021
Yeah that doesn't seem too bad I suppose.