Adds type annotations for ember-glimmer macro implementations #15862
Conversation
| import { assert } from 'ember-debug'; | ||
| import { wrapComponentClassAttribute } from '../utils/bindings'; | ||
| import { dynamicComponentMacro } from './dynamic-component'; | ||
| import { hashToArgs } from './utils'; | ||
| import { OpcodeBuilderDSL } from '@glimmer/runtime'; | ||
| import { unwrap } from '@glimmer/util'; |
There was a problem hiding this comment.
I believe this is stripped in glimmer-vm, but I don’t believe that is true here.
There was a problem hiding this comment.
I had that question too, @chadhietala probably knows that stuff the most intimately.
| let args: ComponentArgs = [params.slice(1), hashToArgs(hash), null, null]; | ||
|
|
||
| // We have to coerce makeComponentDefinition into an `any` here because | ||
| // Glimmer VM is typing this too restrictively as a PathReference when it |
There was a problem hiding this comment.
Can you cross link to the issue in glimmer-vm that you will be opening to track removal of the coercion?
There was a problem hiding this comment.
This API no longer exists in Glimmer VM, so I don't think I will be opening that issue.
There was a problem hiding this comment.
@tomdale - So a future update of glimmer-vm will remove this comment (and what it is doing)? Can we link to anything that will help us understand this when we come back to this code in 6 months? I fear without any pointers to additional things to dig into, this will become yet another murder mystery for our future selves to solve. 🤔
There was a problem hiding this comment.
Awesome, thank you (sorry for the run around).
|
|
||
| if (type === Ops.Get || type === Ops.MaybeLocal) { | ||
| let getExp = values[index]; | ||
| let getExp = expectTupleExpression(values[index]); |
There was a problem hiding this comment.
Would it be better to use @glimmer/util‘s expect?
There was a problem hiding this comment.
expect asserts truthiness, but I need to assert arrayiness.
|
Superseded by #15828. |
This PR adds annotations for the various Ember features implemented as Glimmer VM "macros" (i.e. they tap into the low-level opcode builder API). Previously these were mostly typed as
anys. I skipped typing thecomponenthelper implementation due to time constraints, but it should be straightforward to derive its types from the others I've done here.