Skip to content

fix: add missing SiteConfig service override typings and definitions #281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
vkumar-sonata wants to merge 5 commits into
base: main
Choose a base branch
from
+56 −10
Open

fix: add missing SiteConfig service override typings and definitions #281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea6e34c
fix: add missing SiteConfig service override definitions
vkumar-sonata Jul 21, 2026
3195cdc
fix: correct service constructor types and add typecheck fixture
vkumar-sonata Aug 6, 2026
8c5c1a8
fix: refine service constructor types based on reviewer feedback
vkumar-sonata Aug 31, 2026
2f5cb31
fix: resolve conflicts when upstream from opensource main branch
vkumar-sonata Aug 31, 2026
f817779
fix: resolve AxiosJwtAuthService JSDocs types, fix barrel exports and…
vkumar-sonata Sep 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions runtime/analytics/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export interface AnalyticsService {
sendTrackingLogEvent(eventName: string, properties: object): Promise<unknown>;
identifyAuthenticatedUser(userId: string | number, traits?: Record<string, unknown>): void;
identifyAnonymousUser(traits?: Record<string, unknown>): void;
sendTrackEvent(eventName?: string, properties?: Record<string, unknown>): void;
sendPageEvent(category: string, name: string, properties?: Record<string, unknown>): void;
}
12 changes: 3 additions & 9 deletions runtime/auth/AxiosJwtAuthService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,9 @@ const optionsPropTypes = {
class AxiosJwtAuthService {
/**
* @param {Object} options
* @param {Object} options.config
* @param {string} options.config.baseUrl
* @param {string} options.config.lmsBaseUrl
* @param {string} options.config.loginUrl
* @param {string} options.config.logoutUrl
* @param {string} options.config.refreshAccessTokenApiPath
* @param {string} options.config.accessTokenCookieName
* @param {string} options.config.csrfTokenApiPath
* @param {import('../../types').SiteConfig} options.config
* @param {Object} options.loggingService requires logError and logInfo methods
* @param {Array} [options.middleware] Optional array of middleware functions to apply to the HTTP clients.
*/
constructor(options) {
this.authenticatedHttpClient = null;
Expand Down Expand Up @@ -288,7 +282,7 @@ class AxiosJwtAuthService {
* console.log(authenticatedUser); // Will contain additional user information
* ```
*
* @returns {Promise<null>}
* @returns {Promise<void>}
*/
async hydrateAuthenticatedUser() {
const user = this.getAuthenticatedUser();
Expand Down
15 changes: 15 additions & 0 deletions runtime/auth/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { User } from '../../types';

export interface AuthService {
getAuthenticatedHttpClient(options?: Record<string, unknown>): unknown;
getHttpClient(options?: Record<string, unknown>): unknown;
getLoginRedirectUrl(redirectUrl?: string): string;
redirectToLogin(redirectUrl?: string): void;
getLogoutRedirectUrl(redirectUrl?: string): string;
redirectToLogout(redirectUrl?: string): void;
getAuthenticatedUser(): User | null;
setAuthenticatedUser(authUser: User): void;
fetchAuthenticatedUser(options?: Record<string, unknown>): Promise<User | null>;
ensureAuthenticatedUser(redirectUrl?: string): Promise<User>;
hydrateAuthenticatedUser(): Promise<void>;
}
6 changes: 6 additions & 0 deletions runtime/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export {
sendTrackingLogEvent
} from './analytics';

export type { AnalyticsService } from './analytics/types';

export {
AUTHENTICATED_USER_CHANGED,
AUTHENTICATED_USER_TOPIC,
Expand All @@ -31,6 +33,8 @@ export {
setAuthenticatedUser
} from './auth';

export type { AuthService } from './auth/types';

export {
getSiteConfig,
setSiteConfig,
Expand Down Expand Up @@ -102,6 +106,8 @@ export {
resetLoggingService
} from './logging';

export type { LoggingService } from './logging/types';

export {
CurrentAppContext,
CurrentAppProvider,
Expand Down
26 changes: 25 additions & 1 deletion types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ import { FC, ReactElement, ReactNode } from 'react';
import { MessageDescriptor } from 'react-intl';
import { RouteObject } from 'react-router';
import { SlotOperation } from './runtime/slots/types';
import { LoggingService } from './runtime/logging/types';
import { AnalyticsService } from './runtime/analytics/types';
import { AuthService } from './runtime/auth/types';

// Apps

Expand Down Expand Up @@ -60,6 +63,22 @@ export interface RequiredSiteConfig {
export type LocalizedMessages = Record<string, Record<string, string>>;
export type SiteMessages = LocalizedMessages[];

export type LoggingServiceClass = new (options: {
config: SiteConfig;
}) => LoggingService;

export type AnalyticsServiceClass = new (options: {
config: SiteConfig;
loggingService: LoggingService;
httpClient: unknown;
}) => AnalyticsService;

export type AuthServiceClass = new (options: {
config: SiteConfig;
loggingService: LoggingService;
middleware: unknown[];
}) => AuthService;
Comment on lines +76 to +80

@arbrandes arbrandes Sep 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

authService: AxiosJwtAuthService still doesn't compile. Replace the seven @param {string} options.config.* lines at AxiosJwtAuthService.js:33-44 with @param {import('../../types').SiteConfig} options.config, and add @param {Array} [options.middleware].

That JSDoc types the constructor's options.config as an object with seven required strings. SiteConfig has refreshAccessTokenApiPath, accessTokenCookieName and csrfTokenApiPath as optional, so config: SiteConfig on line 77 isn't assignable to it:

error TS2322: Type 'typeof AxiosJwtAuthService' is not assignable to type 'AuthServiceClass'.
  Types of construct signatures are incompatible.
    The types of 'config.refreshAccessTokenApiPath' are incompatible between these types.
      Type 'string | undefined' is not assignable to type 'string'.

@vkumar-sonata vkumar-sonata Sep 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Updated the JSDoc in AxiosJwtAuthService.js lines 33-44 to replace the seven individual @param {string} options.config.* entries with @param {import('../../types').SiteConfig} options.config and added @param {Array} [options.middleware].


export interface OptionalSiteConfig {
// Site environment
environment: EnvironmentTypes;
Expand Down Expand Up @@ -97,6 +116,11 @@ export interface OptionalSiteConfig {

// Analytics
segmentKey: string | null;

// Services
loggingService: LoggingServiceClass;
analyticsService: AnalyticsServiceClass;
authService: AuthServiceClass;
}

export type SiteConfig = RequiredSiteConfig & Partial<OptionalSiteConfig>;
Expand Down Expand Up @@ -125,7 +149,7 @@ export interface User {
roles: string[];
userId: number;
username: string;
avatar: string;
avatar?: string;
}

export enum EnvironmentTypes {
Expand Down