-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core(tsc): add explicit index signature in mainthread-work-breakdown #5859
Conversation
@@ -5,9 +5,11 @@ | |||
*/ | |||
'use strict'; | |||
|
|||
/** @typedef {'parseHTML'|'styleLayout'|'paintCompositeRender'|'scriptParseCompile'|'scriptEvaluation'|'garbageCollection'|'other'} TaskGroupIds */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been wondering is there not a way to get these programmtically via some keyof
magic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we're going the opposite direction by declaring the type of the object as matching this, but ya know just for knowledge :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been wondering is there not a way to get these programmtically via some keyof magic?
I tried really hard :) but I couldn't find a way to get out of the circularity of needing TaskGroup.id
to be able to index taskGroups
in mainthread-work-breakdown.js
, which means the taskGroups
declaration needs to be typed, not just inferred (it would go to string
), which means you can't meaningfully do keyof
on it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, you could do one really terrible thing...
const taskGroups = {
parseHTML: {
id: /** @type {'parseHTML'} */ ('parseHTML'),
// ...
},
styleLayout: {
id: /** @type {'styleLayout'} */ ('styleLayout'),
// ...
}, // ...
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
alright thanks good to know!
@patrickhulce are you 👍 for this then? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Another change anticipating typescript 3.1nightly. Default index signatures are going away in JS (my fault :), so we'll need to migrate anything relying on that to either fully defined objects or explicit index signatures. We use those in a lot of places, so I thought I'd start now :)
Here
TaskGroupIds
just becomes the object's signature.