[VAULT-34483] update namespace-picker from javascript to typescript#30437
Conversation
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
|
Build Results: |
beagins
force-pushed
the
VAULT-34216/namespace-picker-feature-branch
branch
from
7c57f56 to
3020877
Compare
beagins
force-pushed
the
VAULT-34216/to-typescript
branch
from
5cae053 to
3abe45c
Compare
lane-wetmore
left a comment
lane-wetmore
left a comment
There was a problem hiding this comment.
Great job on this 🎉
| @service declare auth: any; | ||
| @service declare namespace: any; | ||
| @service declare router: any; | ||
| @service declare store: any; |
There was a problem hiding this comment.
there is a type Store from '@ember-data/store', that I see used elsewhere, would that work here?
| @service declare store: any; | |
| @service declare store: Store; |
There was a problem hiding this comment.
Thank you for catching these!! They should all be addressed now :)
| @service store; | ||
| @service declare auth: any; | ||
| @service declare namespace: any; | ||
| @service declare router: any; |
There was a problem hiding this comment.
There is also a type for Router we've used elsewhere, type Router from '@ember/routing/router'
| @service declare router: any; | |
| @service declare router: Router; |
| @service router; | ||
| @service store; | ||
| @service declare auth: any; | ||
| @service declare namespace: any; |
There was a problem hiding this comment.
Would type NamespaceService from 'vault/services/namespace' work for this type instead of any?
| @service namespace; | ||
| @service router; | ||
| @service store; | ||
| @service declare auth: any; |
There was a problem hiding this comment.
would type AuthService from 'vault/services/auth' work for this type instead of any?
|
|
||
| constructor() { | ||
| super(...arguments); | ||
| constructor(owner: unknown, args: any) { |
There was a problem hiding this comment.
Can we define an Args interface for this type instead of any? Something like here
There was a problem hiding this comment.
Good catch! So we aren't currently passing anything into NamespacePicker, so args is empty, and lint complained about an empty interface, and when I looked it up, it should look something like:
constructor(owner: unknown, args: Record<string, never>) {We'll introduce an interface later if/when we add args to NamespacePicker.
beagins
merged this pull request
into
VAULT-34216/namespace-picker-feature-branch
…30437) * [VAULT-34483] updates from js to ts * address PR comments
3 participants
Description
TODO only if you're a HashiCorp employee
backport/label that matches the desired release branch. Note that in the CE repo, the latest release branch will look likebackport/x.x.x, but older release branches will bebackport/ent/x.x.x+ent.of a public function, even if that change is in a CE file, double check that
applying the patch for this PR to the ENT repo and running tests doesn't
break any tests. Sometimes ENT only tests rely on public functions in CE
files.
in the PR description, commit message, or branch name.
description. Also, make sure the changelog is in this PR, not in your ENT PR.