[Service Bus] Move scheduledMessageCount to TopicRuntimeProps from SubscriptionRuntimeProps #10250
Conversation
HarshaNalluru
requested review from
bterlson,
chradek,
qiaozha,
ramya-rao-a,
richardpark-msft and
xirzec
as code owners
ghost
added
the
|
/azp run js - servicebus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| type MakeTOptional<M, T extends keyof M> = Partial<Pick<M, T>> & Omit<M, T>; | ||
| const messageCountDetails: MakeTOptional< | ||
| MessageCountDetails, | ||
| "scheduledMessageCount" | ||
| > = getMessageCountDetails(rawSubscription[Constants.COUNT_DETAILS]); |
There was a problem hiding this comment.
but why...
I tried the below and it builds just fine
const messageCountDetails = getMessageCountDetails(rawSubscription[Constants.COUNT_DETAILS]);
delete messageCountDetails.scheduledMessageCount;There was a problem hiding this comment.
Wait what... How could you?
There was a problem hiding this comment.
Interesting, the build succeeded for me but the squiggly lines are present, say that "The operand of a 'delete' operator must be optional."
There was a problem hiding this comment.
What version of TypeScript is your vscode using? (Should show in the lower right-hand side) Maybe it's a newer one than what the project compiles with.
There was a problem hiding this comment.
So, TS had a change microsoft/TypeScript#37921 to add this check(The operand of a 'delete' operator must be optional.), they reverted it in microsoft/TypeScript#38154 in [email protected] since it broke VSCode(microsoft/vscode#96022).
Planned this check for a later release [email protected] and hence reverted the revert in microsoft/TypeScript#38173 for [email protected].
Technically, we should not be able to delete the non-optional properties moving forward.
There was a problem hiding this comment.
What version of TypeScript is your vscode using? (Should show in the lower right-hand side) Maybe it's a newer one than what the project compiles with.
My VS Code is on typescript@next - Currently, I'm on this version - https://www.npmjs.com/package/typescript/v/4.0.0-dev.20200717
There was a problem hiding this comment.
I am on 3.9.6
Even if we want to future proof ourself, I'd much rather get rid of getMessageCountDetails() and inline that code rather than use the above in order to help readability of code.
HarshaNalluru
changed the title
Issue #10241