Skip to content

Conversation

petamoriken
Copy link
Contributor

@petamoriken petamoriken commented Apr 16, 2021

Fixes #43709
Fixes #32191

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 16, 2021
@ghost
Copy link

ghost commented Apr 16, 2021

CLA assistant check
All CLA requirements met.

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Apr 16, 2021
@andrewbranch andrewbranch requested review from orta and sandersn April 16, 2021 19:35
@petamoriken petamoriken changed the title Mark properties defined in Annex B as deprecated Update lib types to mark Annex B as deprecated Apr 16, 2021
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 16, 2021

You probably have to update baselines.

If we do this, the first thing you'll see every time you request completions on a string is a bunch of crossed-out properties. @sandersn any idea how easy it would be to give them a lower sort order based on JSDoc tags? Ideally, we never would've shown these properties anyway, but we traditionally had to have them there because ES6.

@sandersn
Copy link
Member

@deprecated has its own NodeFlags entry right now, so you just have to use the rules from the checker to decide if the signature as a whole should be considered deprecated (basically, whether all the signature's declarations are deprecated). I don't know the sort order code but I assume it would be easy.

@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #43709. If you can get it accepted, this PR will have a better chance of being reviewed.

@sandersn
Copy link
Member

If we do this, the first thing you'll see every time you request completions on a string is a bunch of crossed-out properties.

A quick grep of the baselines for "big" doesn't look like there's a test for this, so you should add one, something like

/// <reference path='fourslash.ts' />
//// var s = "foo"./*1*/
verify.baselineCompletions()

Which should make it obvious what order "big" et al have.

@DanielRosenwasser
Copy link
Member

I think before this goes in, we need to add the sorting logic first. I'd like to discuss that at an upcoming editor sync with @mjbvz and others first.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 23, 2021

#43796 tracks that.

@DanielRosenwasser
Copy link
Member

CC @bterlson

@petamoriken
Copy link
Contributor Author

@sandersn verify.baselineCompletions method is broken. Your code emits baseline as a global completion such that

[
  {
    "marker": {
      "fileName": "/tests/cases/fourslash/completionsStringMethods.ts",
      "position": 6,
      "name": ""
    },
    "completionList": {
      "isGlobalCompletion": true,
      "isMemberCompletion": false,
      "isNewIdentifierLocation": false,
      "entries": [
        {
          "name": "globalThis",
          "kind": "module",
          "kindModifiers": "",
          "sortText": "5"
        },
        {
          "name": "eval",
          "kind": "function",
          "kindModifiers": "declare",
          "sortText": "5"
        },
        // ...,
      ]
    }
  }
}

I also found the following broken test.

//// foo.newMet/*14*/
verify.baselineCompletions()

"marker": {
"fileName": "/tests/cases/fourslash/completionsJSDocTags.ts",
"position": 1100,
"name": "14"
},
"completionList": {
"isGlobalCompletion": true,
"isMemberCompletion": false,
"isNewIdentifierLocation": false,

@orta
Copy link
Contributor

orta commented Apr 29, 2021

I'm a fan of this, and I think the sorting would make it shine 👍🏻

@sandersn sandersn self-assigned this Apr 29, 2021
@orta
Copy link
Contributor

orta commented May 3, 2021

Sorting PR: #43880

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Aug 28, 2021
@petamoriken
Copy link
Contributor Author

petamoriken commented Aug 28, 2021

Blocked by #45615

@petamoriken
Copy link
Contributor Author

I found that RegExp.$1 to RegExp.$9 should also be deprecated.

@petamoriken
Copy link
Contributor Author

This pull request can be reviewed. Thank you for your time.

Copy link
Contributor

@orta orta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@DanielRosenwasser DanielRosenwasser merged commit d699bcd into microsoft:main Aug 30, 2021
@petamoriken petamoriken deleted the annex-b-as-deprecated branch August 30, 2021 18:05
BobobUnicorn added a commit to BobobUnicorn/TypeScript that referenced this pull request Oct 24, 2021
* Mark properties defined in Annex B as deprecated

* Tweak

* Update baselines

* Update fourslash tests

* Add completionsStringMethods.ts test

* Fix sortText value in fourslash test for deprecated tags

* Update package-lock.json

* Update package-lock.json

* Mark Non-standard RegExp Constructor properties as deprecated

* Update baselines

Co-authored-by: TypeScript Bot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Mark properties defined in Annex B as deprecated escape() function not marked as deprecated in lib.es5.d.ts
5 participants