Fix typo in enum-like code sample #627
Conversation
Deploying contributing-docs with
|
| Latest commit: |
c59d178
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4dcad755.contributing-docs.pages.dev |
| Branch Preview URL: | https://fix-enumlike-example.contributing-docs.pages.dev |
|
Great job, no security vulnerabilities found in this Pull Request |
| export const CipherType = Object.freeze({ | ||
| Login: 1, | ||
| SecureNote: 2, | ||
| Card: 3, | ||
| Identity: 4, | ||
| SshKey: 5, | ||
| } as const); | ||
|
|
||
| export type CipherType = _CipherType[keyof typeof CipherType]; | ||
| export type CipherType = (typeof CipherType)[keyof typeof CipherType]; |
There was a problem hiding this comment.
I was wondering about side-by-side exported const and type declarations with the same name, so I gave it a go. It's still surprising to me that in this case import { CipherType } from "@bitwarden/common/vault/enums"; brings in both concerns, but yeah, it works... nicely!
There was a problem hiding this comment.
I didn't consider this... 🤯 handy though!
There was a problem hiding this comment.
The way that the symbols work is not unlike C namespaces. Both namespaces end up being imported. You don't need to specify them, however, because whether the symbol is in type or data position can be inferred.
There was a problem hiding this comment.
whether the symbol is in type or data position can be inferred
I wonder if there are any cases where it can't be inferred... (I couldn't think of any)? 🤔
| export const CipherType = Object.freeze({ | ||
| Login: 1, | ||
| SecureNote: 2, | ||
| Card: 3, | ||
| Identity: 4, | ||
| SshKey: 5, | ||
| } as const); | ||
|
|
||
| export type CipherType = _CipherType[keyof typeof CipherType]; | ||
| export type CipherType = (typeof CipherType)[keyof typeof CipherType]; |
There was a problem hiding this comment.
Probably worth mentioning, this specific example is not what main currently looks like, and should be updated in the actual code to match this updated example/guidance (once the blocking typescript version is available).
|
Requesting a review from dept-architecture, which is needed to update the ADR. |
🎟️ Tracking
N/A
📔 Objective
Fix what I think is a typo in #624. The code sample refers to
_CipherTypewhich is undefined, I think this is a remnant from the previous code example which used this convention.⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes