MSAL-Node Configuration: Refactoring to remove the ambiguity for deeply nested properties

[Robbie-Microsoft]

Core Library

MSAL Node (@azure/msal-node)

Wrapper Library

Not Applicable

Public or Confidential Client?

Confidential

Description

In TypeScript, the spread operator does not ignore undefined values.

For example, as it relates to msal-node, when building the configuration for an application:

Assuming
DEFAULT_AUTH_OPTIONS.clientCertificate: { thumbprint: Constants.EMPTY_STRING, thumbprintSha256: Constants.EMPTY_STRING, privateKey: Constants.EMPTY_STRING, x5c: Constants.EMPTY_STRING, }
and
auth.clientCertificate = undefined (can be undefined as long as clientSecret or clientAssertion is defined)

Then after
auth: { ...DEFAULT_AUTH_OPTIONS, ...auth },,
We want auth.clientCertificate to be equal to DEFAULT_AUTH_OPTIONS.clientCertificate. However, it's equal to undefined.

This is apparently expected behavior in TypeScript, but is not desired behavior in our case. Therefore, we need to refactor this spread operator functionality here and wherever else it's used when we want to ignore undefined values.

Please see this discussion for additional context.

Source

Internal (Microsoft)

[bgavrilMS]

Sounds good to me to eliminate a source of confusion like this. But how can you "refactor this spread operator functionality" ?

[Robbie-Microsoft]

@bgavrilMS, I should have worded it better. I meant to say "refactor this functionality, in general". For example, one solution would be to not use spread here, and instead write a short algorithm for merging that ignores undefined values.

[bgavrilMS]

Ack, no concern on my part as long as customers can keep using the operator. Sent from Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Robbie-Microsoft ***@***.***> Sent: Saturday, July 27, 2024 12:29:29 AM To: AzureAD/microsoft-authentication-library-for-js ***@***.***> Cc: Mention ***@***.***>; Comment ***@***.***> Subject: Re: [AzureAD/microsoft-authentication-library-for-js] MSAL-Node Configuration: Refactoring to remove the ambiguity for deeply nested properties (Issue #7221) @bgavrilMS<https://github.com/bgavrilMS>, I should have worded it better. I meant to say "refactor this functionality, in general". For example, one solution would be to not use spread here, and instead write a short algorithm for merging that ignores undefined values. — Reply to this email directly, view it on GitHub<#7221 (comment)> or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC5UN2GGDGQMT43HVGZ5BKTZOJTNTBFKMF2HI4TJMJ2XIZLTSWBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVIZDANZTGI3DSNZQG6SG4YLNMWUWQYLTL5WGCYTFNSBKK5TBNR2WLKRUGM3DKNRVGI4TKNVENZQW2ZNJNBQXGX3MMFRGK3FMON2WE2TFMN2F65DZOBS2YSLTON2WKQ3PNVWWK3TUUZ2G64DJMNZZJAVEOR4XAZNKOJSXA33TNF2G64TZUV3GC3DVMWUDQMZQHA2TKNZZQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRSGQZTCMBUGM4TSMUCUR2HS4DFUVWGCYTFNSSXMYLMOVS2UMRQG4ZTENRZG4YDPAVEOR4XAZNFNRQWEZLMUV3GC3DVMWVDIMZWGU3DKMRZGU3KO5DSNFTWOZLSUZRXEZLBORSQ>. You are receiving this email because you were mentioned. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.