Skip to content

Commit e1a94c5

Browse files
odinrCopilot
andauthored
feat: improve module provider validation and dispose handling (#3597)
* feat: improve BaseModuleProvider instanceof validation - Enhance Symbol.hasInstance to validate version as valid semver - Ensure dispose property is a function - Add proper TSDoc documentation - Create changeset for patch release * fix: ensure dispose method proxy binding in AuthProvider - Fix dispose method proxy handling to ensure proper binding when accessed through proxy - Create changeset for patch release * Update packages/modules/module/src/lib/provider/BaseModuleProvider.ts Co-authored-by: Copilot <[email protected]> * docs: add comment explaining use of 'in' operator in Symbol.hasInstance - Clarify why we check prototype chain instead of own properties - Explain duck-typing behavior for instanceof checks * fix: resolve TypeScript errors in BaseModuleProvider Symbol.hasInstance Use Record<string, unknown> type assertion instead of casting to BaseModuleProvider to avoid accessing properties on object type errors reported by reviewdog. --------- Co-authored-by: Copilot <[email protected]>
1 parent dcdec9f commit e1a94c5

File tree

4 files changed

+44
-3
lines changed

4 files changed

+44
-3
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@equinor/fusion-framework-module": patch
3+
---
4+
5+
Improve Symbol.hasInstance validation in BaseModuleProvider to ensure version is a valid semver and dispose is a function.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@equinor/fusion-framework-module-msal": patch
3+
---
4+
5+
Fix dispose method proxy handling in AuthProvider to ensure proper binding when accessed through proxy.

packages/modules/module/src/lib/provider/BaseModuleProvider.ts

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
import { coerce } from 'semver';
2+
import { Subscription, type TeardownLogic } from 'rxjs';
3+
14
import SemanticVersion from '../semantic-version.js';
25

36
import type { IModuleProvider } from './IModuleProvider.js';
47

5-
import { Subscription, type TeardownLogic } from 'rxjs';
6-
78
export type BaseModuleProviderCtorArgs<TConfig = unknown> = {
89
version: string | SemanticVersion;
910
config: TConfig;
@@ -15,6 +16,32 @@ export type BaseModuleProviderCtorArgs<TConfig = unknown> = {
1516
* this is the interface which is returned after enabling a module
1617
*/
1718
export abstract class BaseModuleProvider<TConfig = unknown> implements IModuleProvider {
19+
/**
20+
* Custom instanceof check for module providers.
21+
* Determines if an object is considered an instance of BaseModuleProvider
22+
* by validating the structure and ensuring the version is a valid SemanticVersion
23+
* and dispose is a function.
24+
*
25+
* @param instance - The object to check
26+
* @returns True if the object has valid structure and property types, false otherwise
27+
*/
28+
static [Symbol.hasInstance](instance: unknown): boolean {
29+
const hasStructure =
30+
instance !== null &&
31+
typeof instance === 'object' &&
32+
// Use 'in' operator to check prototype chain, allowing for duck-typing
33+
// where inherited properties are acceptable for instanceof checks
34+
'version' in instance &&
35+
'dispose' in instance;
36+
if (hasStructure) {
37+
const obj = instance as Record<string, unknown>;
38+
const version = coerce(String(obj.version));
39+
const dispose = obj.dispose;
40+
return !!version && typeof dispose === 'function';
41+
}
42+
return false;
43+
}
44+
1845
#version: SemanticVersion;
1946
#subscriptions: Subscription;
2047

packages/modules/msal/src/v2/provider.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,13 @@ export class AuthProvider
142142
}
143143

144144
_createProxyProvider_v2(): IAuthProvider {
145-
return new Proxy(this, {
145+
const proxy = new Proxy(this, {
146146
get: (target: AuthProvider, prop: keyof AuthProvider) => {
147147
switch (prop) {
148148
case 'version':
149149
return target.version;
150+
case 'dispose':
151+
return target.dispose.bind(target);
150152
case 'client':
151153
return target.client;
152154
// @ts-expect-error this is deprecated since version 5.0.1
@@ -170,5 +172,7 @@ export class AuthProvider
170172
}
171173
},
172174
});
175+
176+
return proxy;
173177
}
174178
}

0 commit comments

Comments
 (0)