Add static index signature #37797

Kingwl · 2020-04-05T05:03:28Z

Kingwl · 2020-04-05T17:37:46Z

typescript-bot · 2020-04-05T17:38:50Z

Heya @Kingwl, I've started to run the tarball bundle task on this PR at a5f3d15. You can monitor the build here.

typescript-bot · 2020-04-05T17:51:53Z

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/70462/artifacts?artifactName=tgz&fileId=3AF6CC9DE1A8ED4CFFF5BB83EE19491B3EC20CE91E716546D47CA7344C97A65702&fileName=/typescript-3.9.0-insiders.20200405.tgz"
    }
}

and then running npm install.

orta · 2020-04-08T11:52:30Z

@typescript-bot pack this.

typescript-bot · 2020-04-08T11:52:36Z

Heya @orta, I've started to run the tarball bundle task on this PR at a5f3d15. You can monitor the build here.

typescript-bot · 2020-04-08T12:09:54Z

Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/70679/artifacts?artifactName=tgz&fileId=A68270BE90993FCC02761F5C7DFC4E733CB522BA025C69FCE66DDE502EC10C9B02&fileName=/typescript-3.9.0-insiders.20200408.tgz"
    }
}

and then running npm install.

There is also a playground for this build.

sandersn · 2020-04-15T15:46:30Z

@weswigham the reviewer list is broken on this PR, but you should look at this too

Kingwl · 2020-04-26T09:18:22Z

⬆️

tests/baselines/reference/staticIndexer.errors.txt

tests/baselines/reference/staticIndexers.errors.txt

tests/baselines/reference/staticAsIdentifier.errors.txt

elibarzilay · 2020-04-29T20:17:24Z

src/compiler/checker.ts

@@ -9664,6 +9677,13 @@ namespace ts {
            }
        }

+        function getIndexInfoOfIndexSymbol(indexSymbol: Symbol, indexKind: IndexKind) {


Since this is used only once in the above changed code, maybe make it a local helper instead?

I kinda prefer it this way to avoid several closure allocations, but it's likely a premature optimization.

(I'd be surprised if at least the v8 compiler wouldn't optimize it away...)

elibarzilay · 2020-04-29T20:24:52Z

src/compiler/checker.ts

@@ -11094,24 +11114,29 @@ namespace ts {
        }

        function getIndexSymbol(symbol: Symbol): Symbol | undefined {


You're splitting both getIndexSymbol and getIndexDeclarationOfSymbol to two functions each, because your code calls them with different inputs, and it's the only place that needs the two new entry points. One such refactor seems questionable, but two looks like too much. (But OTOH I can't figure out if there's a way to avoid it.)

elibarzilay · 2020-04-29T20:30:23Z

Ping @DanielRosenwasser, @weswigham.

tests/baselines/reference/staticAsIdentifier.errors.txt

Kingwl · 2020-05-19T14:28:26Z

Hi, Folks, I don't know what I can do. Should I work with the parser issue inside this PR?

DanielRosenwasser · 2020-05-19T18:18:19Z

I would say no - though, if you want to tackle the problem, coordinate with @rbuckton on #38283.

Otherwise, I'm not familiar enough with the internals. @elibarzilay and @weswigham, can you give this another review?

weswigham

The implementation looks good, but I'd love to see some more tests (you should consider placing them in a subfolder of the conformance directory instead of compiler, since it's a "new" language feature). Specifically, I'm thinking of tests showing:

Class static inheritance both inheriting and overriding index signatures (and issuing errors if you override with an incompatible type)
Assignability of class static sides to one another when index signatures are in place (readonly and not readonly)
Assignability between class static sides and equivalent interfaces describing them
Assignability between class static sides with generic static index signatures (this likely requires defining classes within a closure with generic parameters) to have some sanity checks on the behavior of variance digests with these
Some usages with indexed access types, eg (typeof Cls)[string]
Some usages with keyof, eg keyof typeof Cls
Some usages with homomorphic mapped types, eg Extract<typeof Cls, string>

More would also be welcome, ofc, if you can think of any other possible edge cases.

sandersn · 2021-02-16T23:10:36Z

@elibarzilay @weswigham I'd like to get this in early in the 4.3 cycle. Does this (1) have enough tests now (2) error correctly on property types?

Kingwl · 2021-03-04T06:43:17Z

Friendly ping 👀

Kingwl · 2021-03-25T07:31:35Z

👀PiNg AgAiN

weswigham

Assuming my question is answered, I think this is probably good now, with @sandersn 's questions both answering in the affirmative. @elibarzilay since you're the PR assignee, you can probably merge this for this cycle whenever you're ready.

src/compiler/checker.ts

Kingwl · 2021-03-25T16:49:39Z

just for verify

@typescript-bot pack this.

typescript-bot · 2021-03-25T16:49:43Z

Heya @Kingwl, I've started to run the tarball bundle task on this PR at f5508e1. You can monitor the build here.

typescript-bot · 2021-03-25T17:04:11Z

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/99257/artifacts?artifactName=tgz&fileId=E1B419FD818D7F5E2314236B7CDD646F933C3C20811250634D11FB9268A194FA02&fileName=/typescript-4.3.0-insiders.20210325.tgz"
    }
}

and then running npm install.

orta · 2021-03-25T17:24:04Z

Hrm, I'm not certain why the playground didn't build (as this seems to be completely up to date with master), I'll look into add some more logs in the build process.

Kingwl · 2021-03-26T03:29:03Z

Try again.
@typescript-bot pack this.

typescript-bot · 2021-03-26T03:29:21Z

Heya @Kingwl, I've started to run the tarball bundle task on this PR at f5508e1. You can monitor the build here.

typescript-bot · 2021-03-26T03:48:22Z

Hey @Kingwl, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/99342/artifacts?artifactName=tgz&fileId=9F9DC5C2BFBC2F804A9BB87AAC7535FF4F449685817397BB01F4B3AB0D61A51C02&fileName=/typescript-4.3.0-insiders.20210326.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

Kingwl force-pushed the static_index branch from f4f354c to a5f3d15 Compare April 5, 2020 05:52

Kingwl changed the title ~~Add static index~~ Add static index signature Apr 6, 2020

sandersn added the For Backlog Bug PRs that fix a backlog bug label Apr 15, 2020

sandersn assigned elibarzilay Apr 15, 2020

elibarzilay suggested changes Apr 29, 2020

View reviewed changes

elibarzilay requested a review from weswigham April 29, 2020 20:29

DanielRosenwasser requested changes Apr 29, 2020

View reviewed changes

tests/baselines/reference/staticAsIdentifier.errors.txt Outdated Show resolved Hide resolved

Kingwl force-pushed the static_index branch from 7e5091d to 6d8ed7b Compare April 30, 2020 09:33

weswigham requested changes May 20, 2020

View reviewed changes

Kingwl added 6 commits May 20, 2020 21:34

Add static index

ed1a798

fix lint

f83b7de

make lint happy

d6fdab8

adjust test cases

8eca2be

add more cases

b7f614f

fix changes

d4624e6

Kingwl force-pushed the static_index branch from 2d61217 to d4624e6 Compare May 20, 2020 13:36

Kingwl added 3 commits January 20, 2021 00:12

fix crash

0ee41f6

Accept baseline

f91d84b

Merge branch 'master' into static_index

aaf5185

DanielRosenwasser mentioned this pull request Feb 11, 2021

TypeScript 4.3 Iteration Plan #42762

Closed

Kingwl added 3 commits March 4, 2021 13:01

Merge branch 'master' into static_index

f8eddee

Fix regression

246585f

Fix crash

124e461

RyanCavanaugh added the Rescheduled This issue was previously scheduled to an earlier milestone label Mar 4, 2021

RyanCavanaugh modified the milestones: TypeScript 4.2.0, TypeScript 4.3.0 Mar 4, 2021

weswigham approved these changes Mar 25, 2021

View reviewed changes

src/compiler/checker.ts Show resolved Hide resolved

Kingwl added 2 commits March 25, 2021 16:12

Merge branch 'master' into static_index

23cbc31

always return new array

f5508e1

Kingwl requested a review from DanielRosenwasser March 26, 2021 06:55

DanielRosenwasser approved these changes Mar 26, 2021

View reviewed changes

DanielRosenwasser merged commit 41dc625 into microsoft:master Mar 26, 2021

Kingwl deleted the static_index branch June 30, 2021 08:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add static index signature #37797

Add static index signature #37797

Kingwl commented Apr 5, 2020 •

edited

Loading

Kingwl commented Apr 5, 2020

typescript-bot commented Apr 5, 2020 •

edited

Loading

typescript-bot commented Apr 5, 2020

orta commented Apr 8, 2020

typescript-bot commented Apr 8, 2020 •

edited

Loading

typescript-bot commented Apr 8, 2020 •

edited

Loading

sandersn commented Apr 15, 2020

Kingwl commented Apr 26, 2020

elibarzilay Apr 29, 2020

DanielRosenwasser Apr 29, 2020

elibarzilay Apr 29, 2020

elibarzilay Apr 29, 2020

elibarzilay commented Apr 29, 2020

Kingwl commented May 19, 2020

DanielRosenwasser commented May 19, 2020

weswigham left a comment

sandersn commented Feb 16, 2021

Kingwl commented Mar 4, 2021

Kingwl commented Mar 25, 2021

weswigham left a comment

Kingwl commented Mar 25, 2021

typescript-bot commented Mar 25, 2021 •

edited

Loading

typescript-bot commented Mar 25, 2021

orta commented Mar 25, 2021

Kingwl commented Mar 26, 2021

typescript-bot commented Mar 26, 2021 •

edited

Loading

typescript-bot commented Mar 26, 2021 •

edited

Loading

		@@ -11094,24 +11114,29 @@ namespace ts {
		}

		function getIndexSymbol(symbol: Symbol): Symbol \| undefined {

Add static index signature #37797

Add static index signature #37797

Conversation

Kingwl commented Apr 5, 2020 • edited Loading

Kingwl commented Apr 5, 2020

typescript-bot commented Apr 5, 2020 • edited Loading

typescript-bot commented Apr 5, 2020

orta commented Apr 8, 2020

typescript-bot commented Apr 8, 2020 • edited Loading

typescript-bot commented Apr 8, 2020 • edited Loading

sandersn commented Apr 15, 2020

Kingwl commented Apr 26, 2020

elibarzilay Apr 29, 2020

Choose a reason for hiding this comment

DanielRosenwasser Apr 29, 2020

Choose a reason for hiding this comment

elibarzilay Apr 29, 2020

Choose a reason for hiding this comment

elibarzilay Apr 29, 2020

Choose a reason for hiding this comment

elibarzilay commented Apr 29, 2020

Kingwl commented May 19, 2020

DanielRosenwasser commented May 19, 2020

weswigham left a comment

Choose a reason for hiding this comment

sandersn commented Feb 16, 2021

Kingwl commented Mar 4, 2021

Kingwl commented Mar 25, 2021

weswigham left a comment

Choose a reason for hiding this comment

Kingwl commented Mar 25, 2021

typescript-bot commented Mar 25, 2021 • edited Loading

typescript-bot commented Mar 25, 2021

orta commented Mar 25, 2021

Kingwl commented Mar 26, 2021

typescript-bot commented Mar 26, 2021 • edited Loading

typescript-bot commented Mar 26, 2021 • edited Loading

Kingwl commented Apr 5, 2020 •

edited

Loading

typescript-bot commented Apr 5, 2020 •

edited

Loading

typescript-bot commented Apr 8, 2020 •

edited

Loading

typescript-bot commented Apr 8, 2020 •

edited

Loading

typescript-bot commented Mar 25, 2021 •

edited

Loading

typescript-bot commented Mar 26, 2021 •

edited

Loading

typescript-bot commented Mar 26, 2021 •

edited

Loading