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 iOShttps://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)

@bgavrilMShttps://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 GitHubhttps://github.com/AzureAD/microsoft-authentication-library-for-js/issues/7221#issuecomment-2253003884 or unsubscribehttps://github.com/notifications/unsubscribe-auth/AC5UN2GGDGQMT43HVGZ5BKTZOJTNTBFKMF2HI4TJMJ2XIZLTSWBKK5TBNR2WLJDUOJ2WLJDOMFWWLO3UNBZGKYLEL5YGC4TUNFRWS4DBNZ2F6YLDORUXM2LUPGBKK5TBNR2WLJDUOJ2WLJDOMFWWLLTXMF2GG2C7MFRXI2LWNF2HTAVFOZQWY5LFUVUXG43VMWSG4YLNMWVXI2DSMVQWIX3UPFYGLAVFOZQWY5LFVIZDANZTGI3DSNZQG6SG4YLNMWUWQYLTL5WGCYTFNSBKK5TBNR2WLKRUGM3DKNRVGI4TKNVENZQW2ZNJNBQXGX3MMFRGK3FMON2WE2TFMN2F65DZOBS2YSLTON2WKQ3PNVWWK3TUUZ2G64DJMNZZJAVEOR4XAZNKOJSXA33TNF2G64TZUV3GC3DVMWUDQMZQHA2TKNZZQKSHI6LQMWSWS43TOVS2K5TBNR2WLKRSGQZTCMBUGM4TSMUCUR2HS4DFUVWGCYTFNSSXMYLMOVS2UMRQG4ZTENRZG4YDPAVEOR4XAZNFNRQWEZLMUV3GC3DVMWVDIMZWGU3DKMRZGU3KO5DSNFTWOZLSUZRXEZLBORSQ. You are receiving this email because you were mentioned.

Triage notifications on the go with GitHub Mobile for iOShttps://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Androidhttps://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.