diff --git a/core-web/CLAUDE.md b/core-web/CLAUDE.md index d72e48a9a7c9..725e31fb8f88 100644 --- a/core-web/CLAUDE.md +++ b/core-web/CLAUDE.md @@ -98,6 +98,46 @@ Always wrap form fields with this structure for consistent styling: ``` +## TypeScript Strict Mode + +Strict mode is being rolled out **one project at a time** (epic #35932), bottom-up through the dependency graph. `tsconfig.base.json` stays at `"strict": false` — never flip it globally. + +To make a project strict: + +1. Add the flags to the **project's own** `tsconfig.json` (not `tsconfig.spec.json`, not the base): + + ```json + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true + ``` + +2. Fix every error. No new `any` — use explicit types. To silence something unavoidable, use `@ts-expect-error` with a `// TODO(#issue):` note, never a blanket `@ts-ignore`. + +**What enforces this:** for Rollup libs that emit declarations (`"declaration": true`), `@rollup/plugin-typescript` is in the build chain and reports type errors, so the `build` target is the gate — CI runs `nx run-many -t build` (the `build-test` execution in `core-web/pom.xml`). Do **not** add a separate `typecheck` target to those projects; it is redundant. `lint` does not catch type errors — ESLint reports lint rules, not TS diagnostics. + +Vite-based projects are the exception: their builds use esbuild and skip type checking, which is why the Nx Vite plugin infers a separate `typecheck` target for them. + +Verify locally: + +```bash +pnpm exec tsc -p /tsconfig.lib.json --noEmit +pnpm exec nx run :build +pnpm exec nx affected -t build,lint --base=origin/main # check you didn't break consumers +``` + +`` is the path from `project.json`, which is often nested — e.g. `libs/sdk/create-app`, not `libs/create-app`. Two caveats on the tsconfig name: + +- **Apps** use `tsconfig.app.json`. +- **Some projects have no `tsconfig.lib.json`** (`libs/sdk/create-app` is one); use their `tsconfig.json` instead. + +Also check `tsconfig.spec.json` — the flags live in `tsconfig.json`, which the spec config extends, so specs go strict too and their errors are yours to fix. + +> **Watch out for masked results.** If a tsconfig declares a `types` entry that is not installed, `tsc` reports `TS2688: Cannot find type definition file for ''` and **stops before semantic checking** — you get one error and no type checking at all. A stable error count across a change proves nothing in that case. `libs/utils-testing` is affected today (`"types": ["jasmine"]`); check it with `--types node` to see real diagnostics. + ## Portlet Development New portlets go in `libs/portlets/`. For full patterns, architecture, testing, and Nx generator setup: @@ -110,8 +150,8 @@ New portlets go in `libs/portlets/`. For full patterns, architecture, testing, a - Use `dot-content-drive` portlet as reference for test config - `tsconfig.spec.json` tsconfig.spec.json must have "isolatedModules": true in compilerOptions -- `tsconfig.json` — do NOT add `"strict": true` or `"module": "preserve"` -- `tsconfig.spec.json` — keep minimal (only `module`, `target`, `types`) +- `tsconfig.json` — do NOT add `"module": "preserve"` +- `tsconfig.spec.json` — keep minimal (only `module`, `target`, `types`); do NOT add `"strict": true` here, it belongs in the project's `tsconfig.json` (see [TypeScript Strict Mode](#typescript-strict-mode)) - Import `mockProvider` from `@openng/spectator/jest` (not `@openng/spectator`) ### SignalStore Tests diff --git a/core-web/libs/dotcms-js/src/lib/core/api-root.service.ts b/core-web/libs/dotcms-js/src/lib/core/api-root.service.ts index d81908f11082..a4c05ea14198 100644 --- a/core-web/libs/dotcms-js/src/lib/core/api-root.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/api-root.service.ts @@ -12,9 +12,9 @@ export class ApiRoot { hideFireOn = false; hideRulePushOptions = false; - static parseQueryParam(query: string, token: string): string { + static parseQueryParam(query: string, token: string): string | null { let idx = -1; - let result = null; + let result: string | null = null; token = token + '='; if (query && query.length) { idx = query.indexOf(token); diff --git a/core-web/libs/dotcms-js/src/lib/core/dotcms-config.service.ts b/core-web/libs/dotcms-js/src/lib/core/dotcms-config.service.ts index 24bb19f552a6..05c06ef12471 100644 --- a/core-web/libs/dotcms-js/src/lib/core/dotcms-config.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/dotcms-config.service.ts @@ -122,7 +122,8 @@ export class DotcmsConfigService { private http = inject(HttpClient); private loggerService = inject(LoggerService); - private configParamsSubject: BehaviorSubject = new BehaviorSubject(null); + private configParamsSubject: BehaviorSubject = + new BehaviorSubject(null); private configUrl: string; /** @@ -138,7 +139,7 @@ export class DotcmsConfigService { getConfig(): Observable { return this.configParamsSubject .asObservable() - .pipe(filter((config: ConfigParams) => !!config)); + .pipe(filter((config): config is ConfigParams => !!config)); } loadConfig(): void { @@ -159,8 +160,8 @@ export class DotcmsConfigService { paginatorLinks: res.config[DOTCMS_PAGINATOR_LINKS], paginatorRows: res.config[DOTCMS_PAGINATOR_ROWS], releaseInfo: { - buildDate: res.config.releaseInfo?.buildDate, - version: res.config.releaseInfo?.version + buildDate: res.config.releaseInfo?.buildDate ?? '', + version: res.config.releaseInfo?.version ?? '' }, websocket: { websocketReconnectTime: diff --git a/core-web/libs/dotcms-js/src/lib/core/logger.service.ts b/core-web/libs/dotcms-js/src/lib/core/logger.service.ts index 83783ccb5c4a..d1135cc6c1b7 100644 --- a/core-web/libs/dotcms-js/src/lib/core/logger.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/logger.service.ts @@ -63,7 +63,7 @@ export class LoggerService { * @returns boolean */ shouldShowLogs(): boolean { - const devMode: string = this.httpRequestUtils.getQueryStringParam(DEV_MODE_PARAM); + const devMode: string | null = this.httpRequestUtils.getQueryStringParam(DEV_MODE_PARAM); return !environment.production || devMode === 'on'; } // isProduction. @@ -77,13 +77,14 @@ export class LoggerService { try { throw new Error(); } catch (e) { - caller = this.cleanCaller(this.stringUtils.getLine(e.stack, 4)); + const stack = e instanceof Error ? (e.stack ?? '') : ''; + caller = this.cleanCaller(this.stringUtils.getLine(stack, 4)); } return caller; } - private cleanCaller(caller: string): string { + private cleanCaller(caller: string | null): string { return caller ? caller.trim().substr(3) : 'unknown'; } } diff --git a/core-web/libs/dotcms-js/src/lib/core/login.service.ts b/core-web/libs/dotcms-js/src/lib/core/login.service.ts index 4e11ac53174c..cfd0e95df4af 100644 --- a/core-web/libs/dotcms-js/src/lib/core/login.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/login.service.ts @@ -38,23 +38,23 @@ export class LoginService { currentUserLanguageId = ''; private country = ''; private lang = ''; - private urls: Record; + // Typed by inference rather than `Record` so each endpoint is a named + // property; that keeps dot access valid under `noPropertyAccessFromIndexSignature`. + private readonly urls = { + changePassword: '/api/v1/changePassword', + getAuth: '/api/v1/authentication/logInUser', + loginAs: '/api/v1/users/loginas', + logout: '/api/v1/logout', + logoutAs: '/api/v1/users/logoutas', + recoverPassword: '/api/v1/forgotpassword', + serverInfo: '/api/v1/loginform', + userAuth: '/api/v1/authentication', + current: '/api/v1/users/current/' + }; constructor() { this._loginAsUsersList$ = new Subject(); - this.urls = { - changePassword: '/api/v1/changePassword', - getAuth: '/api/v1/authentication/logInUser', - loginAs: '/api/v1/users/loginas', - logout: '/api/v1/logout', - logoutAs: '/api/v1/users/logoutas', - recoverPassword: '/api/v1/forgotpassword', - serverInfo: '/api/v1/loginform', - userAuth: '/api/v1/authentication', - current: '/api/v1/users/current/' - }; - this.dotcmsEventsService.subscribeTo('SESSION_DESTROYED').subscribe(() => { this.logOutUser(); this.clearExperimentPersistence(); @@ -77,7 +77,11 @@ export class LoginService { return this._logout$.asObservable(); } - private _auth: Auth; + // TODO(#35939): assigned by `setAuth()` during the login flow, never in the constructor. + // Modelling it as `Auth | undefined` is the truthful type, but the public `auth` getter is + // consumed by already-strict projects (global-store, data-access), so widening it is a + // public-API change that belongs in its own issue. + private _auth!: Auth; get auth(): Auth { return this._auth; @@ -291,7 +295,7 @@ export class LoginService { this._auth = this.getFullAuth(auth); this._auth$.next(this.getFullAuth(auth)); - this.currentUserLanguageId = auth.user.languageId; + this.currentUserLanguageId = auth.user.languageId ?? ''; // When not logged user we need to fire the observable chain if (!auth.user) { @@ -373,6 +377,8 @@ export interface User { export interface Auth { user: User; - loginAsUser: User; + // Null whenever nobody is impersonating. Callers already guard with + // `auth.loginAsUser || auth.user`; the type just never said so. + loginAsUser: User | null; isLoginAs?: boolean; } diff --git a/core-web/libs/dotcms-js/src/lib/core/routing.service.ts b/core-web/libs/dotcms-js/src/lib/core/routing.service.ts index 037fb990323c..bc4a4665e2f2 100644 --- a/core-web/libs/dotcms-js/src/lib/core/routing.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/routing.service.ts @@ -17,10 +17,10 @@ export class RoutingService { private http = inject(HttpClient); private _menusChange$: Subject = new Subject(); - private menus: Menu[]; + private menus: Menu[] = []; private urlMenus: string; private portlets: Map; - private _currentPortletId: string; + private _currentPortletId = ''; private _portletUrlSource$ = new Subject(); private _currentPortlet$ = new Subject(); @@ -55,7 +55,7 @@ export class RoutingService { return this._portletUrlSource$.asObservable(); } - get firstPortlet(): string { + get firstPortlet(): string | null { const porlets = this.portlets.entries().next().value; return porlets ? porlets[0] : null; @@ -65,7 +65,7 @@ export class RoutingService { this.portlets.set(portletId.replace(' ', '_'), url); } - public getPortletURL(portletId: string): string { + public getPortletURL(portletId: string): string | undefined { return this.portlets.get(portletId); } diff --git a/core-web/libs/dotcms-js/src/lib/core/shared/user.model.ts b/core-web/libs/dotcms-js/src/lib/core/shared/user.model.ts index 2af96dff3bbd..86b047265ae8 100644 --- a/core-web/libs/dotcms-js/src/lib/core/shared/user.model.ts +++ b/core-web/libs/dotcms-js/src/lib/core/shared/user.model.ts @@ -6,8 +6,8 @@ import { LoggerService } from '../logger.service'; export class UserModel { private loggerService = inject(LoggerService); - username: string; - password: string; + username = ''; + password = ''; locale: string; suppressAlerts = false; diff --git a/core-web/libs/dotcms-js/src/lib/core/site.service.mock.ts b/core-web/libs/dotcms-js/src/lib/core/site.service.mock.ts index 7764eee72860..e48edc89b41e 100644 --- a/core-web/libs/dotcms-js/src/lib/core/site.service.mock.ts +++ b/core-web/libs/dotcms-js/src/lib/core/site.service.mock.ts @@ -20,7 +20,7 @@ export const mockSites: Site[] = [ ]; export class SiteServiceMock { - _currentSite: Site; + _currentSite: Site | undefined; private _currentSite$: Subject = new Subject(); get currentSite(): Site { diff --git a/core-web/libs/dotcms-js/src/lib/core/site.service.ts b/core-web/libs/dotcms-js/src/lib/core/site.service.ts index ba41cee5a0c0..50b925231872 100644 --- a/core-web/libs/dotcms-js/src/lib/core/site.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/site.service.ts @@ -31,7 +31,10 @@ export class SiteService { private http = inject(HttpClient); private loggerService = inject(LoggerService); - private selectedSite: Site; + // TODO(#35939): assigned by `setCurrentSite()` during init, never in the constructor. + // Same trade-off as `LoginService._auth`: the public `currentSite` getter is widely + // consumed, so widening it to `Site | undefined` belongs in its own issue. + private selectedSite!: Site; private urls: { currentSiteUrl: string; sitesUrl: string; switchSiteUrl: string }; private events: string[] = [ 'SAVE_SITE', @@ -174,7 +177,7 @@ export class SiteService { * @return {*} {Observable} * @memberof SiteService */ - switchSiteById(id: string): Observable { + switchSiteById(id: string): Observable { this.loggerService.debug('Applying a Site Switch'); return this.getSiteById(id).pipe( diff --git a/core-web/libs/dotcms-js/src/lib/core/string-utils.service.ts b/core-web/libs/dotcms-js/src/lib/core/string-utils.service.ts index a55d97af7d6c..288e9008e0f9 100644 --- a/core-web/libs/dotcms-js/src/lib/core/string-utils.service.ts +++ b/core-web/libs/dotcms-js/src/lib/core/string-utils.service.ts @@ -10,8 +10,8 @@ export class StringUtils { * @param indexLine * @returns string */ - getLine(text: string, indexLine: number): string { - let line: string = null; + getLine(text: string, indexLine: number): string | null { + let line: string | null = null; if (text) { const lines = text.split('\n'); @@ -26,9 +26,9 @@ export class StringUtils { * @param str * @returns string */ - camelize(str): string { + camelize(str: string): string { return str - .replace(/(?:^\w|[A-Z]|\b\w)/g, (letter, index) => { + .replace(/(?:^\w|[A-Z]|\b\w)/g, (letter: string, index: number) => { return index === 0 ? letter.toLowerCase() : letter.toUpperCase(); }) .replace(/\s+/g, ''); diff --git a/core-web/libs/dotcms-js/src/lib/core/util/http-request-utils.ts b/core-web/libs/dotcms-js/src/lib/core/util/http-request-utils.ts index 39ff02220c0a..32b90e217916 100644 --- a/core-web/libs/dotcms-js/src/lib/core/util/http-request-utils.ts +++ b/core-web/libs/dotcms-js/src/lib/core/util/http-request-utils.ts @@ -25,8 +25,8 @@ export class HttpRequestUtils { * it is based on the window.location.href. * @returns string */ - getQueryStringParam(name: string): string { - let value = null; + getQueryStringParam(name: string): string | null { + let value: string | null = null; const regex = new RegExp('[?&]' + name.replace(/[\[\]]/g, '\\$&') + '(=([^&#]*)|&|#|$)'); const results = regex.exec(window.location.href); diff --git a/core-web/libs/dotcms-js/src/lib/core/util/response-view.ts b/core-web/libs/dotcms-js/src/lib/core/util/response-view.ts index 44246596a2e3..ebe234610a0f 100644 --- a/core-web/libs/dotcms-js/src/lib/core/util/response-view.ts +++ b/core-web/libs/dotcms-js/src/lib/core/util/response-view.ts @@ -15,47 +15,44 @@ import { DotCMSResponse } from '@dotcms/dotcms-models'; * */ export class ResponseView { - private bodyJsonObject: DotCMSResponse; + // `HttpResponse.body` is nullable, so the parsed body genuinely can be absent. + private bodyJsonObject: DotCMSResponse | null; private headers: HttpHeaders; public constructor(private resp: HttpResponse>) { - try { - this.bodyJsonObject = resp.body; - this.headers = resp.headers; - } catch (e) { - this.bodyJsonObject = null; - } + this.bodyJsonObject = resp.body; + this.headers = resp.headers; } - public header(headerName: string): string { + public header(headerName: string): string | null { return this.headers.get(headerName); } get i18nMessagesMap(): { [key: string]: string } { - return this.bodyJsonObject.i18nMessagesMap; + return this.bodyJsonObject?.i18nMessagesMap ?? {}; } - get contentlets(): T { - return this.bodyJsonObject.contentlets; + get contentlets(): T | undefined { + return this.bodyJsonObject?.contentlets; } - get entity(): T { - return this.bodyJsonObject.entity; + get entity(): T | undefined { + return this.bodyJsonObject?.entity; } - get tempFiles(): T { - return this.bodyJsonObject.tempFiles; + get tempFiles(): T | undefined { + return this.bodyJsonObject?.tempFiles; } get errorsMessages(): string { let errorMessages = ''; - if (this.bodyJsonObject.errors) { + if (this.bodyJsonObject?.errors) { this.bodyJsonObject.errors.forEach((e: any) => { errorMessages += e.message; }); } else { - errorMessages = this.bodyJsonObject.messages.toString(); + errorMessages = this.bodyJsonObject?.messages.toString() ?? ''; } return errorMessages; @@ -71,7 +68,7 @@ export class ResponseView { public existError(errorCode: string): boolean { return ( - this.bodyJsonObject.errors && + !!this.bodyJsonObject?.errors && this.bodyJsonObject.errors.filter((e: any) => e.errorCode === errorCode).length > 0 ); } diff --git a/core-web/libs/dotcms-js/tsconfig.json b/core-web/libs/dotcms-js/tsconfig.json index d72ba3eb3c86..d2cd03a27e5a 100644 --- a/core-web/libs/dotcms-js/tsconfig.json +++ b/core-web/libs/dotcms-js/tsconfig.json @@ -14,6 +14,12 @@ "target": "es2020", "module": "preserve", "moduleResolution": "bundler", - "lib": ["dom", "dom.iterable", "es2022"] + "lib": ["dom", "dom.iterable", "es2022"], + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true } } diff --git a/core-web/libs/sdk/create-app/src/index.ts b/core-web/libs/sdk/create-app/src/index.ts index 871c759a176c..12d39bf892c9 100644 --- a/core-web/libs/sdk/create-app/src/index.ts +++ b/core-web/libs/sdk/create-app/src/index.ts @@ -390,7 +390,7 @@ program if (error instanceof Error) { console.error(error.message); // Preserve stack trace for debugging when DEBUG mode is enabled - if (process.env.DEBUG) { + if (process.env['DEBUG']) { console.error('\n' + chalk.gray('Stack trace:')); console.error(chalk.gray(error.stack || 'No stack trace available')); } diff --git a/core-web/libs/sdk/create-app/src/utils/index.ts b/core-web/libs/sdk/create-app/src/utils/index.ts index e2955be7d806..4903ffe34e49 100644 --- a/core-web/libs/sdk/create-app/src/utils/index.ts +++ b/core-web/libs/sdk/create-app/src/utils/index.ts @@ -116,6 +116,13 @@ export async function fetchWithRetry( await new Promise((r) => setTimeout(r, delay)); } } + + // Only reachable when retries < 1, in which case the loop never runs. Throwing keeps the + // return type free of `undefined` and surfaces the bad argument instead of hiding it. + // Note `retries` is the total attempt count, not the number of retries after the first try. + throw new Error( + chalk.red(`\n❌ fetchWithRetry requires at least 1 attempt, received ${retries}\n`) + ); } export function getUVEConfigValue(frontEndUrl: string) { diff --git a/core-web/libs/sdk/create-app/tsconfig.json b/core-web/libs/sdk/create-app/tsconfig.json index f644ba6f2136..8a433ca61775 100644 --- a/core-web/libs/sdk/create-app/tsconfig.json +++ b/core-web/libs/sdk/create-app/tsconfig.json @@ -1,7 +1,13 @@ { "extends": "../../../tsconfig.base.json", "compilerOptions": { - "module": "esnext" + "module": "esnext", + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true }, "files": [], "include": [], diff --git a/core-web/libs/utils-testing/src/lib/dot-content-types.mock.ts b/core-web/libs/utils-testing/src/lib/dot-content-types.mock.ts index d7d76df1723a..8e0637c3ad9f 100644 --- a/core-web/libs/utils-testing/src/lib/dot-content-types.mock.ts +++ b/core-web/libs/utils-testing/src/lib/dot-content-types.mock.ts @@ -69,7 +69,10 @@ export const dotcmsContentTypeBasicMock = { } as unknown as DotCMSContentType; export const dotcmsContentTypeFieldBasicMock: DotCMSContentTypeField = { - ...EMPTY_SYSTEM_FIELD + ...EMPTY_SYSTEM_FIELD, + // `EMPTY_SYSTEM_FIELD` is `Omit` — a partial template — so a + // concrete class is supplied here. Callers that care override it. + clazz: DotCMSClazzes.TEXT }; export const fieldsWithBreakColumn: DotCMSContentTypeLayoutRow[] = [ diff --git a/core-web/libs/utils/src/lib/dot-utils.spec.ts b/core-web/libs/utils/src/lib/dot-utils.spec.ts index 20af5ac34a1f..55863a4ae502 100644 --- a/core-web/libs/utils/src/lib/dot-utils.spec.ts +++ b/core-web/libs/utils/src/lib/dot-utils.spec.ts @@ -310,8 +310,8 @@ describe('Dot Utils', () => { it('should handle null currentUrl and requestHostName', () => { const url = 'https://example.com/{requestHostName}{currentUrl}{urlSearchParams}'; const params: DotPageToolUrlParams = { - currentUrl: null, // Handle null by substituting with empty string - requestHostName: null, // Handle null by substituting with empty string + currentUrl: null as unknown as string, // Handle null by substituting with empty string + requestHostName: null as unknown as string, // Handle null by substituting with empty string siteId: '', languageId: 1 }; @@ -324,8 +324,8 @@ describe('Dot Utils', () => { const params: DotPageToolUrlParams = { currentUrl: '', requestHostName: '', - siteId: null, // Handle null by not appending the query parameter - languageId: null // Handle null by not appending the query parameter + siteId: null as unknown as string, // Handle null by not appending the query parameter + languageId: null as unknown as number // Handle null by not appending the query parameter }; expect(getRunnableLink(url, params)).toEqual('https://example.com/page'); @@ -334,10 +334,10 @@ describe('Dot Utils', () => { it('should handle all parameters as null or empty', () => { const url = 'https://example.com/{requestHostName}{currentUrl}{urlSearchParams}'; const params: DotPageToolUrlParams = { - currentUrl: null, // Handle null by substituting with empty string - requestHostName: null, // Handle null by substituting with empty string - siteId: null, // Handle null by not appending the query parameter - languageId: null // Handle null by not appending the query parameter + currentUrl: null as unknown as string, // Handle null by substituting with empty string + requestHostName: null as unknown as string, // Handle null by substituting with empty string + siteId: null as unknown as string, // Handle null by not appending the query parameter + languageId: null as unknown as number // Handle null by not appending the query parameter }; expect(getRunnableLink(url, params)).toEqual('https://example.com/'); diff --git a/core-web/libs/utils/src/lib/dot-utils.ts b/core-web/libs/utils/src/lib/dot-utils.ts index 750cb2b3bed5..3fe821c52035 100644 --- a/core-web/libs/utils/src/lib/dot-utils.ts +++ b/core-web/libs/utils/src/lib/dot-utils.ts @@ -127,18 +127,18 @@ export function getRunnableLink(url: string, currentPageUrlParams: DotPageToolUr */ export function getImageAssetUrl(contentlet: DotCMSContentlet): string { if (!contentlet?.baseType) { - return contentlet.asset; + return contentlet['asset']; } switch (contentlet?.baseType) { case DotCMSBaseTypesContentTypes.FILEASSET: - return contentlet.fileAssetVersion || contentlet.fileAsset; + return contentlet['fileAssetVersion'] || contentlet['fileAsset']; case DotCMSBaseTypesContentTypes.DOTASSET: - return contentlet.assetVersion || contentlet.asset; + return contentlet['assetVersion'] || contentlet['asset']; default: - return contentlet?.asset || ''; + return contentlet?.['asset'] || ''; } } @@ -149,8 +149,12 @@ export function getImageAssetUrl(contentlet: DotCMSContentlet): string { * @param limit - The maximum length of the truncated text. * @returns The truncated text with ellipsis if it exceeds the limit, otherwise the original text. */ -export function ellipsizeText(text: string, limit: number): string { - if (!text || typeof text !== 'string' || limit <= 0 || isNaN(limit)) { +export function ellipsizeText( + text: string | null | undefined, + limit: number | null | undefined +): string { + // `limit == null` is checked explicitly so the remaining comparisons narrow it to `number`. + if (!text || typeof text !== 'string' || limit == null || limit <= 0 || isNaN(limit)) { return ''; } diff --git a/core-web/libs/utils/src/lib/services/dot-asset.service.ts b/core-web/libs/utils/src/lib/services/dot-asset.service.ts index 9ab930607283..0af67de83fdb 100644 --- a/core-web/libs/utils/src/lib/services/dot-asset.service.ts +++ b/core-web/libs/utils/src/lib/services/dot-asset.service.ts @@ -5,7 +5,7 @@ import { DotHttpErrorResponse } from '@dotcms/dotcms-models'; -export const fallbackErrorMessages = { +export const fallbackErrorMessages: { [key: number]: string } = { 500: '500 Internal Server Error', 400: '400 Bad Request', 401: '401 Unauthorized Error' @@ -19,7 +19,7 @@ export const fallbackErrorMessages = { export function createDotAsset( options: DotAssetCreateOptions ): Promise { - const promises = []; + const promises: Promise[] = []; let filesCreated = 1; options.files.map((file: DotCMSTempFile) => { const data = { @@ -70,7 +70,7 @@ export function createDotAsset( }); } -function fetchAsset(url, data): Promise { +function fetchAsset(url: string, data: unknown): Promise { return fetch(url, { method: 'PUT', headers: { diff --git a/core-web/libs/utils/src/lib/shared/FieldUtil.ts b/core-web/libs/utils/src/lib/shared/FieldUtil.ts index 7e455f30f1ac..6df86eee2ecd 100644 --- a/core-web/libs/utils/src/lib/shared/FieldUtil.ts +++ b/core-web/libs/utils/src/lib/shared/FieldUtil.ts @@ -6,33 +6,37 @@ import { DotCMSDataTypes } from '@dotcms/dotcms-models'; -export const EMPTY_FIELD: DotCMSContentTypeField = { +/** + * Blank template for a content type field. `clazz` is deliberately omitted: `DotCMSClazz` is a + * union of concrete implementation class names with no "empty" member, and every derived constant + * supplies its own. Spread this and add `clazz` to obtain a full `DotCMSContentTypeField`. + */ +export const EMPTY_FIELD: Omit = { contentTypeId: '', - dataType: null, + dataType: '', fieldType: '', fieldTypeLabel: '', fieldVariables: [], - fixed: null, - iDate: null, - id: null, - indexed: null, - listed: null, - modDate: null, - name: null, - readOnly: null, - required: null, - searchable: null, - sortOrder: null, - unique: null, - variable: null, - clazz: null, - defaultValue: null, - hint: null, + fixed: false, + iDate: 0, + id: '', + indexed: false, + listed: false, + modDate: 0, + name: '', + readOnly: false, + required: false, + searchable: false, + sortOrder: 0, + unique: false, + variable: '', + defaultValue: undefined, + hint: undefined, regexCheck: undefined, - values: null + values: undefined }; -export const EMPTY_SYSTEM_FIELD: DotCMSContentTypeField = { +export const EMPTY_SYSTEM_FIELD: Omit = { ...EMPTY_FIELD, dataType: DotCMSDataTypes.SYSTEM }; @@ -235,22 +239,28 @@ export class FieldUtil { * @memberof FieldUtil */ static getFieldsWithoutLayout(layout: DotCMSContentTypeLayoutRow[]): DotCMSContentTypeField[] { - return layout - .map((row: DotCMSContentTypeLayoutRow) => row.columns) - .filter((columns: DotCMSContentTypeLayoutColumn[]) => !!columns) - .reduce( - ( - accumulator: DotCMSContentTypeLayoutColumn[], - currentValue: DotCMSContentTypeLayoutColumn[] - ) => accumulator.concat(currentValue), - [] - ) - .map((fieldColumn) => fieldColumn.fields) - .reduce( - (accumulator: DotCMSContentTypeField[], currentValue: DotCMSContentTypeField[]) => - accumulator.concat(currentValue), - [] - ); + return ( + layout + .map((row: DotCMSContentTypeLayoutRow) => row.columns) + // Type guard rather than a plain truthy filter: `columns` is optional on the row, and + // only a predicate signature narrows it away for the `reduce` below. + .filter((columns): columns is DotCMSContentTypeLayoutColumn[] => !!columns) + .reduce( + ( + accumulator: DotCMSContentTypeLayoutColumn[], + currentValue: DotCMSContentTypeLayoutColumn[] + ) => accumulator.concat(currentValue), + [] + ) + .map((fieldColumn) => fieldColumn.fields) + .reduce( + ( + accumulator: DotCMSContentTypeField[], + currentValue: DotCMSContentTypeField[] + ) => accumulator.concat(currentValue), + [] + ) + ); } /** diff --git a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts index 6a4a63bbba72..a91bc3e7d0da 100644 --- a/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts +++ b/core-web/libs/utils/src/lib/shared/contentlet.utils.spec.ts @@ -64,14 +64,14 @@ describe('utils', () => { }; const result = getFileMetadata(contentlet); - expect(result).toEqual(contentlet.metaData); + expect(result).toEqual(contentlet['metaData']); }); it('should return assetMetaData if metaData is not present', () => { const contentlet: DotCMSContentlet = NEW_FILE_MOCK.entity; const result = getFileMetadata(contentlet); - expect(result).toEqual(contentlet.assetMetaData); + expect(result).toEqual(contentlet['assetMetaData']); }); it('should return an empty object if neither metaData nor assetMetaData is present', () => { @@ -97,7 +97,7 @@ describe('utils', () => { const contentlet: DotCMSContentlet = { ...NEW_FILE_MOCK.entity }; - delete contentlet.assetVersion; + delete contentlet['assetVersion']; const result = getFileVersion(contentlet); expect(result).toBeNull(); @@ -141,7 +141,7 @@ describe('utils', () => { ...TEMP_FILE_MOCK, mimeType: 'image/jpeg' }; - const acceptedFiles = []; + const acceptedFiles: string[] = []; expect(checkMimeType(file, acceptedFiles)).toBe(true); }); @@ -166,7 +166,7 @@ describe('utils', () => { it('returns false for file with no mime type', () => { const file = { ...TEMP_FILE_MOCK, - mimeType: null + mimeType: null as unknown as string }; const acceptedFiles = ['image/jpeg']; expect(checkMimeType(file, acceptedFiles)).toBe(false); diff --git a/core-web/libs/utils/tsconfig.json b/core-web/libs/utils/tsconfig.json index d72ba3eb3c86..d2cd03a27e5a 100644 --- a/core-web/libs/utils/tsconfig.json +++ b/core-web/libs/utils/tsconfig.json @@ -14,6 +14,12 @@ "target": "es2020", "module": "preserve", "moduleResolution": "bundler", - "lib": ["dom", "dom.iterable", "es2022"] + "lib": ["dom", "dom.iterable", "es2022"], + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true } } diff --git a/specs/35939-dotcms-js-strict-mode/spec.md b/specs/35939-dotcms-js-strict-mode/spec.md new file mode 100644 index 000000000000..5e37bdfa6d11 --- /dev/null +++ b/specs/35939-dotcms-js-strict-mode/spec.md @@ -0,0 +1,235 @@ +# Spec: Enable TypeScript strict mode in `dotcms-js` + +**Issue:** [#35939](https://github.com/dotCMS/core/issues/35939) — [06/44] · **Epic:** [#35932](https://github.com/dotCMS/core/issues/35932) +**Status:** Awaiting review (Phase 1 — Specify) + +--- + +## Objective + +Enable TypeScript `strict` mode for the Nx project `dotcms-js` (`core-web/libs/dotcms-js`) and resolve the **38 type errors** it surfaces, without introducing new `any` and without breaking any of its **20 dependent projects**. + +**Who benefits:** the ~20 downstream projects — including the `dotcms-ui` admin app — that compile `dotcms-js` source directly through the `@dotcms/dotcms-js` path alias. Today its null-safety holes are invisible to them; after this change the types tell the truth. + +**Why it matters here specifically:** `dotcms-js` is a layer-1 core library (auth, site, routing, config, HTTP response wrapping). Six of its consumers are *already* strict, so its loose types are actively leaking `any`-shaped uncertainty into projects that have opted into rigour. + +### Assumptions (validated with the requester) + +1. Follow precedent **#36879** (`dotcms-models`): the six flags go in the project's own `tsconfig.json`. `tsconfig.base.json` stays at `"strict": false` — never flipped globally. +2. No `typescript-strict-plugin`, no `tsc-strict` script, no `// @ts-strict-ignore`. That approach was dropped; the bootstrap #35933 closed without the plugin landing. Sub-issue ACs referencing them are stale. +3. **Scope is `tsconfig.lib.json` only.** `tsconfig.spec.json` fails today with `TS2688: Cannot find type definition file for 'jasmine'` — a pre-existing, non-strict-related breakage. Out of scope. +4. **The `skip:lint` / `skip:test` tags are not touched.** `nx run dotcms-js:lint` currently fails with **42 problems** (41 errors, mostly `no-explicit-any`). Re-enabling lint is a separate effort. +5. Legacy packaging debt (`ng-package.json`, `tslint.json`, peerDeps pinned to Angular `^6.0.0 || ^7.2.0`) is left as-is. + +### Accepted trade-off: strict will be unenforced + +**Decision made by the requester: enable strict only — no `typecheck` target, no CI gate.** + +Recorded plainly so it is not rediscovered later: `dotcms-js` has **no `build` target**. Its only targets are `lint`, `test`, and `nx-release-publish`, and the first two are tag-excluded from CI. Consumers compile it via path mapping under *their own* tsconfig, so the flags added here are read by nothing in CI. + +Consequence: after this work, `tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit` will be clean, but **nothing prevents the next commit from regressing it**. The flags document intent; they do not enforce it. + +Partial mitigation that comes for free: the six already-strict consumers (below) will surface *some* regressions in their own builds, because they compile this source strictly. That coverage is incidental and incomplete — it only catches errors on code paths those six actually import. + +--- + +## Tech Stack + +| | | +|---|---| +| Language | TypeScript 6.0.3 | +| Framework | Angular 21+ (this library is Angular services + models, `@Injectable`) | +| Monorepo | Nx 23, pnpm 10.17.1, Node 22.22.3 | +| Test runner | Karma + Jasmine (3 spec files; target is tag-excluded from CI) | + +--- + +## Commands + +```bash +cd core-web + +# Measure the current error surface (the core loop for this work) +pnpm exec tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit + +# Before the flags land, simulate them on the CLI +pnpm exec tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit \ + --strict --noImplicitOverride --noPropertyAccessFromIndexSignature \ + --noImplicitReturns --noFallthroughCasesInSwitch --forceConsistentCasingInFileNames + +# Verify the six already-strict consumers still compile +pnpm exec nx run-many -t build,lint -p data-access,global-store,portlets-dot-analytics,portlets-dot-analytics-data-access,portlets-dot-locales-portlet,utils-testing + +# Full blast-radius check across all 20 dependents +pnpm exec nx affected -t build,lint --base=origin/main --exclude=tag:skip:lint + +# Formatting gate +pnpm exec nx format:check --base=origin/main +``` + +> Environment note: `pnpm` is not on `PATH` by default in this worktree. Use `corepack pnpm` with Node 22.22.3 from nvm (`.nvmrc`). + +--- + +## Project Structure + +``` +core-web/libs/dotcms-js/ +├── src/ +│ ├── public_api.ts → Public barrel (what the 20 consumers import) +│ └── lib/core/ +│ ├── login.service.ts → 12 errors — largest cluster +│ ├── util/response-view.ts → 6 errors — HTTP response wrapper +│ ├── string-utils.service.ts → 5 errors +│ ├── routing.service.ts → 4 errors +│ ├── dotcms-config.service.ts → 3 errors +│ ├── site.service.ts → 2 errors +│ ├── shared/user.model.ts → 2 errors — PUBLIC MODEL, handle with care +│ ├── site.service.mock.ts → 1 error +│ ├── logger.service.ts → 1 error +│ ├── api-root.service.ts → 1 error +│ └── util/http-request-utils.ts → 1 error +├── tsconfig.json → WHERE THE SIX FLAGS GO +├── tsconfig.lib.json → extends tsconfig.json; the compilation unit in scope +└── tsconfig.spec.json → out of scope (pre-existing jasmine failure) +``` + +31 `.ts` files, 3 `.spec.ts`. Errors touch 11 files. + +--- + +## Code Style + +The six flags, added to `libs/dotcms-js/tsconfig.json` — identical to precedent #36879: + +```json +{ + "extends": "../../tsconfig.base.json", + "compilerOptions": { + "target": "es2020", + "module": "preserve", + "moduleResolution": "bundler", + "lib": ["dom", "dom.iterable", "es2022"], + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true + } +} +``` + +Fix style — model reality, do not silence the compiler: + +```ts +// GOOD — the value genuinely can be absent, so say so +getCookie(name: string): string | null { + return this.readCookie(name); +} + +// BAD — hides the hole the flag just exposed +getCookie(name: string): string { + return this.readCookie(name) as string; +} + +// GOOD — index-signature access (TS4111), purely mechanical +this.urls['current'] + +// GOOD — uninitialized field that is genuinely set later +private _auth: Auth | null = null; + +// BAD — definite-assignment assertion papering over real absence +private _auth!: Auth; +``` + +**Never** add `any`, `@ts-ignore`, or `!` non-null assertions to clear an error. If a value is truly always present, prove it with initialization or a constructor assignment. + +--- + +## Testing Strategy + +**There is effectively no test safety net here, and the plan must not pretend otherwise.** + +- 3 `.spec.ts` files exist, but the `test` target is tag-excluded (`skip:test`) and `tsconfig.spec.json` does not even compile (pre-existing jasmine types failure). +- Therefore verification is **compilation-based**, not test-based. + +| Level | Mechanism | What it proves | +|---|---|---| +| Unit | — | Nothing. No usable suite. | +| Type | `tsc -p tsconfig.lib.json --noEmit` | The 38 errors are gone | +| Integration | `nx run-many -t build,lint` on the 6 strict consumers | Public-surface changes did not break rigorous consumers | +| System | `nx affected -t build,lint` over all 20 dependents | No regression anywhere downstream | + +Writing new tests is **out of scope** — the suite cannot run without first fixing the jasmine types breakage. + +--- + +## Boundaries + +**Always:** +- Run the full `nx affected -t build,lint` before opening the PR — 20 projects depend on this library +- Prefer fixes that widen types honestly (`string | null`) over fixes that assert away the problem +- Keep each fix minimal and local to the error site + +**Ask first:** +- Any change to `src/public_api.ts` (the public barrel) +- Any change to `shared/user.model.ts` — it is a public model consumed downstream; making `username`/`password` optional alters the shape 20 projects see +- Any change that requires editing a *consumer* project to compile +- Removing the `skip:lint` / `skip:test` tags + +**Never:** +- Add `any`, `@ts-ignore`, or `!` non-null assertions to silence a flag +- Touch `core-web/tsconfig.base.json` +- Modify `tsconfig.spec.json` or attempt to fix the jasmine breakage in this PR +- Commit with any of the 20 dependents failing to build + +--- + +## The 38 errors, grouped by fix strategy + +| # | Group | Count | Files | Risk to consumers | +|---|---|---|---|---| +| A | `TS4111` index-signature dot access | 8 | `login.service.ts` (all) | **None** — internal, mechanical `.x` → `['x']` | +| B | `TS2564` uninitialized class property | 8 | `login.service.ts`, `routing.service.ts` ×2, `user.model.ts` ×2, `site.service.mock.ts`, `site.service.ts`, `response-view.ts` | **Medium** — `user.model.ts` is public | +| C | `TS2322`/`TS2345` null & undefined mismatches | 21 | `response-view.ts` ×5, `string-utils.service.ts` ×2, `login.service.ts` ×3, `dotcms-config.service.ts` ×3, `routing.service.ts` ×2, others | **High** — widens public return types | +| D | `TS7006` implicit any parameter | 3 | `string-utils.service.ts` | None — internal callback params | +| E | `TS18046` `unknown` in catch | 1 | `logger.service.ts:80` | None | + +Group A is the safe warm-up. Group C is where the judgement lives. + +--- + +## Success Criteria + +1. `libs/dotcms-js/tsconfig.json` contains all six flags, byte-identical in spirit to #36879. +2. `pnpm exec tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit` exits **0**. +3. **Zero** new `any`, `@ts-ignore`, `@ts-expect-error`, or `!` non-null assertions introduced. Verify with a diff grep. +4. All six already-strict consumers build and lint green: `data-access`, `global-store`, `portlets-dot-analytics`, `portlets-dot-analytics-data-access`, `portlets-dot-locales-portlet`, `utils-testing`. +5. `pnpm exec nx affected -t build,lint --base=origin/main --exclude=tag:skip:lint` exits **0** across all 20 dependents. +6. `pnpm exec nx format:check --base=origin/main` exits **0**. +7. `tsconfig.spec.json` is untouched, and its pre-existing jasmine failure is unchanged (not newly introduced, not fixed). +8. No change to `src/public_api.ts` without explicit approval. + +--- + +## Risks + +| Risk | Likelihood | Mitigation | +|---|---|---| +| Widening a public return type to `\| null` breaks one of the 6 strict consumers | **High** | They are compiled explicitly in the verification loop, before `affected`. Fix forward in the same PR if small; escalate if it cascades. | +| `user.model.ts` shape change ripples across 20 projects | Medium | Listed under "Ask first". Prefer initializing (`username = ''`) over making optional, to preserve the shape. | +| Strict regresses silently after merge | **Certain, accepted** | Out of scope by decision. The six strict consumers give partial, incidental coverage. | +| A fix hides a real bug instead of surfacing it | Medium | The "never assert away" rule in Boundaries; review each Group C fix against actual runtime behaviour. | + +--- + +## Resolved Decisions + +1. **`user.model.ts` → initialize, do not make optional.** `username = ''` and `password = ''`. This preserves the public shape, so the 20 consumers see no type change. Making them optional would be more honest about runtime reality but is not worth the blast radius here. +2. **`response-view.ts` → do not grow this PR.** If widening its getters cascades into the six strict consumers beyond a trivial fix, stop, revert that file's changes, and open a follow-up issue. The PR ships the other groups rather than absorbing a cascade. +3. **Stale ACs on #35939 will be corrected on the issue**, same as was done for epic #35932 — removing the `typescript-strict-plugin`, `npx tsc-strict`, and `// @ts-strict-ignore` criteria that no longer apply. + +## Open Questions + +None outstanding. Ready for Phase 2 (Plan). diff --git a/specs/35941-sdk-uve-strict-mode/spec.md b/specs/35941-sdk-uve-strict-mode/spec.md new file mode 100644 index 000000000000..c884a1d1529b --- /dev/null +++ b/specs/35941-sdk-uve-strict-mode/spec.md @@ -0,0 +1,204 @@ +# Spec: Enable TypeScript strict mode in `sdk-uve` + +**Issue:** [#35941](https://github.com/dotCMS/core/issues/35941) — [08/44] · **Epic:** [#35932](https://github.com/dotCMS/core/issues/35932) +**Status:** Awaiting review (Phase 1 — Specify) +**Conclusion:** **No code change required. The issue is already satisfied.** + +--- + +## Objective + +Verify whether `sdk-uve` (`core-web/libs/sdk/uve`, package `@dotcms/uve`) meets the strict-mode bar defined by the rollout, and close the issue with evidence rather than producing a change for its own sake. + +**Result of the investigation: it already does — and unlike some earlier projects in this rollout, it is genuinely enforced.** + +### Assumptions (stated for correction) + +1. The rollout's definition of "strict" is the six flags from precedent #36879, in the project's own `tsconfig.json`. No `typescript-strict-plugin`, no `tsc-strict`, no `// @ts-strict-ignore` — that approach was dropped. +2. "Done" means: flags present **and** zero errors **and** something in CI actually verifies it. The third clause is the one that separated `sdk-types` (done) from `dotcms-js` / `utils` (declared but unenforced). +3. The 2 commits by which this branch trails `origin/main` are irrelevant to the conclusion, but the branch should be updated before any work. + +--- + +## Evidence + +### 1. The flags are already there + +`libs/sdk/uve/tsconfig.json` carries all six: + +```json +"forceConsistentCasingInFileNames": true, +"strict": true, +"noImplicitOverride": true, +"noPropertyAccessFromIndexSignature": true, +"noImplicitReturns": true, +"noFallthroughCasesInSwitch": true +``` + +Present since `277cbbc8f7` — *"chore(SDK): Create `getUVEState` to manage UVE state headlessly (#31242)"* — i.e. from early in the library's life, not added by this rollout. + +### 2. It compiles clean + +| Config | Errors | +|---|---:| +| `tsconfig.lib.json` | **0** | +| `tsconfig.spec.json` | **0** | + +### 3. The code is genuinely clean, not clean-by-escape-hatch + +Across **4518 lines** in 21 `.ts` files (5 of them specs): + +| Pattern | Count | +|---|---:| +| `: any` / `` / `any[]` | **0** | +| `@ts-ignore` / `@ts-expect-error` | **0** | +| Non-null assertions (`!.`) | **0** | + +So the zero-error result is not propped up by suppressions. + +### 4. It is enforced — verified at the source, not assumed + +This is the clause that failed for `dotcms-js` and `utils`, so it was checked directly rather than inferred. + +`libs/sdk/uve/rollup.config.cjs` sets `compiler: 'babel'`, which earlier in this rollout was **wrongly** read as "no type checking". `compiler` governs only the transpile step. `@nx/rollup`'s `withNx` **always** inserts a TypeScript plugin — see `@nx/rollup/src/plugins/with-nx/with-nx.js:164-196`: + +```js +options.useLegacyTypescriptPlugin !== false + ? require('rollup-plugin-typescript2')({ + check: !options.skipTypeCheck, // ← type checking + tsconfig: tsConfigPath, ... }) + : require('@rollup/plugin-typescript')({ + ..., + noEmitOnError: !options.skipTypeCheck }) // ← fails the build +``` + +`sdk-uve` sets neither `skipTypeCheck` nor `useLegacyTypescriptPlugin`, so it gets `rollup-plugin-typescript2` with `check: true` against `tsconfig.lib.json` — the config that carries the strict flags. A strict violation fails the build. + +The same mechanism was **proven empirically** on the sibling project `sdk-types` in this PR: reverting a fix there made `nx run sdk-types:build` fail with `@rollup/plugin-typescript TS2564`. + +### 5. That build runs in CI, and in the release pipeline + +- `project.json` has `"tags": []` — **no `skip:build` / `skip:lint` / `skip:test`.** +- CI runs `nx run-many -t build --exclude=tag:skip:build` (`build-test` in `core-web/pom.xml`), so `sdk-uve` is built on every PR. +- `@dotcms/uve` is a **published npm package** and matches the `sdk-*` glob in the SDK release pipeline, so the same type-checked build gates every release. + > The local `package.json` says `1.1.1`, but that is **not** what ships. The release action rewrites the version to the dotCMS release tag (ADR-0019 date lockstep); npm `latest` is **26.8.7-1** across 197 published versions. + +There are in fact **three** type-checking paths, two of which run in CI: + +| Path | Type-checks? | In CI? | +|---|---|---| +| `build` (rollup) | Yes — TS plugin with `noEmitOnError: !skipTypeCheck` | **Yes**, and the `build-test` execution in `core-web/pom.xml:191` has **no `` element** — it cannot be turned off | +| `test` (ts-jest) | Yes — `diagnostics` is not disabled in `jest.preset.js` nor the project config, so it defaults on, against `tsconfig.spec.json` | **Yes**, via `nx affected -t test` (no skip tag) | +| `build:js` (esbuild) | Yes — `compiler: "tsc"`, `skipTypeCheck` unset → runs `tsc --noEmit` over `tsconfig.lib.json` | No — the target name does not match `nx run-many -t build`, and its output is committed by hand | + +`build-test` being unskippable is what makes this the strongest-enforced project of the rollout so far: `-DskipTests=true` disables `unit-test` and `-Pvalidate` gates `lint-test`, but neither can disable the build. + +### 6. Consumers + +7 dependents; **5 already strict**: + +| Consumer | Strict? | +|---|---| +| `sdk-analytics`, `sdk-angular`, `sdk-experiments`, `sdk-react`, `sdk-vue` | **Yes** | +| `dotcms-ui`, `portlets-edit-ema-portlet` | Inherits `false` | + +Nothing to widen, so no blast radius to manage. + +--- + +## Comparison with the rest of the rollout + +| Issue | Project | Flags | Clean | Enforced | Outcome | +|---|---|:--:|:--:|:--:|---| +| #35935 | `sdk-types` | ✅ | ✅ | ✅ | No-op; shipped docs | +| #35936 | `dotcms` | ❌ | ❌ | ❌ | Dead code → #36950 | +| #35937 | `dot-layout-grid` | ❌ | ❌ | ❌ | Dead code → #36950 | +| #35938 | `sdk-create-app` | ❌ | 2 errors | ✅ | Fixed | +| #35939 | `dotcms-js` | ❌ | 38 errors | ❌ | Fixed, unenforced | +| #35940 | `utils` | ❌ | 32+17 errors | ❌ | Fixed, unenforced | +| **#35941** | **`sdk-uve`** | ✅ | ✅ | ✅ | **No-op** | + +`sdk-uve` is the second project in the rollout that was already finished before the epic started. + +--- + +## Adjacent observation — committed artifact that CI never regenerates + +The `build:js` target emits a file that is **committed to git**, but the target is not invoked by `core-web/pom.xml` or any workflow: + +| Project | Committed artifact | +|---|---| +| `sdk-client` | `dotCMS/src/main/webapp/html/js/editor-js/sdk-editor.js` | +| `sdk-uve` | `dotCMS/src/main/webapp/ext/uve/dot-uve.js` | + +If the source changes and nobody runs `build:js` by hand, the committed file silently drifts out of sync with it, and nothing in CI notices. Out of scope for the strict-mode rollout, but worth a ticket. + +## Commands + +```bash +cd core-web + +pnpm exec tsc -p libs/sdk/uve/tsconfig.lib.json --noEmit # expect 0 +pnpm exec tsc -p libs/sdk/uve/tsconfig.spec.json --noEmit # expect 0 +pnpm exec nx run sdk-uve:build --skip-nx-cache # expect pass +pnpm exec nx run sdk-uve:lint +pnpm exec nx run sdk-uve:test +``` + +> Node 22.22.3 via nvm; `pnpm` is not on `PATH` in this worktree, use `corepack pnpm`. + +## Project Structure + +``` +core-web/libs/sdk/uve/ +├── src/ 21 .ts files, 4518 lines (5 specs) +│ ├── index.ts public barrel → @dotcms/uve +│ ├── internal.ts internal barrel → @dotcms/uve/internal +│ ├── types.ts → @dotcms/uve/types +│ ├── lib/core/, lib/editor/ +│ └── script/sdk-editor.ts entry for build:js → dotCMS webapp +├── tsconfig.json ← the six flags already live here +├── tsconfig.lib.json declaration: true ← what makes rollup type-check +└── rollup.config.cjs compiler: 'babel' (transpile only; TS plugin is separate) +``` + +## Code Style + +Not applicable — no code is being written. If a future change touches this project, the existing bar is: no `any`, no `@ts-ignore`, no `!` assertions, all currently at zero. + +## Testing Strategy + +No new tests. Existing `sdk-uve:test` and `sdk-uve:lint` targets run in CI (no skip tags) and must stay green. Verification for this issue is compilation- and build-based, per the commands above. + +## Boundaries + +**Always:** back the "already done" claim with reproducible commands in the issue comment. + +**Ask first:** any change to `libs/sdk/uve` source — it is a published package with 5 strict consumers, and nothing here needs changing. + +**Never:** add flags that are already present, or make a cosmetic change purely to have a diff for the issue. + +--- + +## Success Criteria + +1. `tsc -p libs/sdk/uve/tsconfig.lib.json --noEmit` → 0 errors. +2. `tsc -p libs/sdk/uve/tsconfig.spec.json --noEmit` → 0 errors. +3. `nx run sdk-uve:build` passes, and the rollup TS-plugin evidence above is recorded. +4. `git diff origin/main -- core-web/libs/sdk/uve` stays **empty** — the deliverable is a verdict, not a diff. +5. #35941 is closed with the evidence, and its stale ACs (`typescript-strict-plugin`, `npx tsc-strict`, `// @ts-strict-ignore`) corrected first — same treatment as #35939. + +## Resolved + +**How to record the closure:** close #35941 directly with the evidence, rather than adding `Closes #35941` to PR #36957. There is no diff to attach, and linking it would imply this PR resolved it — it did not; the project has been compliant since February 2025. + +## Incidental finding — out of scope + +`core-web/tsconfig.base.json:104` maps `@dotcms/uve/types` → `libs/sdk/uve/src/types.ts`, **a file that does not exist**. Nothing imports that specifier, and it is absent from the `exports` / `typesVersions` maps in `libs/sdk/uve/package.json`. A dead alias, unrelated to strict mode. Not touched here — worth a separate cleanup ticket. + +## Steps + +1. Correct the stale ACs on #35941 (`typescript-strict-plugin`, `npx tsc-strict`, `// @ts-strict-ignore`), same treatment as #35939. +2. Comment on #35941 with the evidence table and the reproducible commands. +3. Close it as completed — the acceptance criteria are met, just not by this rollout. +4. Commit `spec.md` alongside the one for #35939 already in PR #36957, so the "already compliant" verdict is recorded for the remaining 36 issues. diff --git a/specs/35942-sdk-client-strict-mode/spec.md b/specs/35942-sdk-client-strict-mode/spec.md new file mode 100644 index 000000000000..0f0b0f3558e0 --- /dev/null +++ b/specs/35942-sdk-client-strict-mode/spec.md @@ -0,0 +1,164 @@ +# Spec: Enable TypeScript strict mode in `sdk-client` + +**Issue:** [#35942](https://github.com/dotCMS/core/issues/35942) — [09/44] · **Epic:** [#35932](https://github.com/dotCMS/core/issues/35932) +**Status:** Awaiting review (Phase 1 — Specify) +**Conclusion:** **No code change required. The issue is already satisfied, and enforced.** + +--- + +## Objective + +Verify whether `sdk-client` (`core-web/libs/sdk/client`, package `@dotcms/client`) meets the rollout's strict-mode bar, and close the issue with evidence rather than manufacturing a diff. + +It does. This is the **third** project in the rollout that was already compliant before the epic began — and the clearest of the three, because its build uses `compiler: 'tsc'` outright. + +### Assumptions + +1. "Strict" means the six flags from precedent #36879, in the project's own `tsconfig.json`. No `typescript-strict-plugin` / `tsc-strict` / `@ts-strict-ignore` — that approach was dropped. +2. "Done" requires three things, not one: flags present **and** zero errors **and** something in CI that actually verifies it. The third clause is what separated `sdk-types` / `sdk-uve` (done) from `dotcms-js` / `utils` (declared but unenforced). + +--- + +## Evidence + +### 1. Flags already present + +`libs/sdk/client/tsconfig.json` carries all six (`strict`, `forceConsistentCasingInFileNames`, `noImplicitOverride`, `noPropertyAccessFromIndexSignature`, `noImplicitReturns`, `noFallthroughCasesInSwitch`). + +This is almost certainly the **origin** of the pattern across the SDK: `git log --follow` on `libs/sdk/uve/tsconfig.json` showed it was created as a `C100` (100%-identical) copy of *this* file. + +### 2. Compiles clean + +| Config | Errors | +|---|---:| +| `tsconfig.lib.json` | **0** | +| `tsconfig.spec.json` | **0** | + +### 3. Clean without escape hatches + +Production source only (specs excluded), across **9600 lines** in 48 `.ts` files (15 of them specs): + +| Pattern | Count | +|---|---:| +| `: any` / `` / `any[]` | **0** | +| `@ts-ignore` / `@ts-expect-error` | **0** | +| Non-null assertions (`!.`) | **0** | + +### 4. Enforced — and here it is unambiguous + +`libs/sdk/client/rollup.config.cjs`: + +```js +compiler: 'tsc', // ← not 'babel' +tsConfig: './tsconfig.lib.json' // ← the config carrying the strict flags +``` + +`skipTypeCheck` is not set anywhere. Unlike `sdk-uve` — where `compiler: 'babel'` made this look ambiguous until the `@nx/rollup` source confirmed the TypeScript plugin is inserted unconditionally — here the build compiles with `tsc` directly against the strict config. A strict violation fails the build. + +Targets, all green on a fresh no-cache run: + +| Target | Result | +|---|---| +| `nx run sdk-client:build` | pass | +| `nx run sdk-client:lint` | pass | +| `nx run sdk-client:test` | pass | + +### 5. That build runs in CI and gates every release + +- `project.json` has `"tags": []` — no `skip:build` / `skip:lint` / `skip:test`. +- CI runs `nx run-many -t build --exclude=tag:skip:build` via the `build-test` execution in `core-web/pom.xml`, which has **no `` element** — `-DskipTests` and `-Pvalidate` cannot disable it. +- `@dotcms/client` is published to npm and matches the `sdk-*` glob in the SDK release pipeline (`cicd_release-sdk.yml` → `deploy-javascript-sdk`), so the same type-checked build gates every release. + > The local `package.json` says `1.2.0`, but that is **not** what ships. The release action rewrites the version to the dotCMS release tag (ADR-0019 date lockstep); npm `latest` is **26.8.7-1** across 262 published versions. Do not quote the local version as the published one. + +### 6. Consumers + +6 dependents; **5 already strict**: + +| Consumer | Strict? | +|---|---| +| `sdk-angular`, `sdk-react`, `sdk-vue`, `sdk-experiments`, `sdk-create-app` | **Yes** | +| `portlets-edit-ema-portlet` | Inherits `false` — and it imports from `@dotcms/client/internal` (`dot-page-api.service.ts:8`) | + +Nothing is being widened, so there is no blast radius. + +### 7. Path aliases all resolve + +Unlike the dangling `@dotcms/uve/types` found in #35941, every alias here points at a real file: + +| Alias | Target | Exists | +|---|---|:--:| +| `@dotcms/client` | `libs/sdk/client/src/index.ts` | ✅ | +| `@dotcms/client/internal` | `libs/sdk/client/src/internal.ts` | ✅ | +| `@dotcms/query-builder` | `libs/sdk/client/src/lib/client/content/builders/query/query.ts` | ✅ | + +--- + +## Rollout status after this issue + +| Issue | Project | Flags | Clean | Enforced | Outcome | +|---|---|:--:|:--:|:--:|---| +| #35935 | `sdk-types` | ✅ | ✅ | ✅ | No-op | +| #35936 | `dotcms` | ❌ | ❌ | ❌ | Dead → #36950 | +| #35937 | `dot-layout-grid` | ❌ | ❌ | ❌ | Dead → #36950 | +| #35938 | `sdk-create-app` | ❌ | 2 err | ✅ | Fixed | +| #35939 | `dotcms-js` | ❌ | 38 err | ❌ | Fixed, unenforced | +| #35940 | `utils` | ❌ | 32+17 err | ❌ | Fixed, unenforced | +| #35941 | `sdk-uve` | ✅ | ✅ | ✅ | No-op | +| **#35942** | **`sdk-client`** | ✅ | ✅ | ✅ | **No-op** | + +Emerging pattern worth noting for the remaining 35: **every `libs/sdk/*` project is already strict and already enforced**, because they share a tsconfig lineage and all build through Nx executors that type-check. The genuinely unfinished work is concentrated in the non-SDK libraries and apps. + +--- + +## Adjacent observation — committed artifact that CI never regenerates + +The `build:js` target emits a file that is **committed to git**, but the target is not invoked by `core-web/pom.xml` or any workflow: + +| Project | Committed artifact | +|---|---| +| `sdk-client` | `dotCMS/src/main/webapp/html/js/editor-js/sdk-editor.js` | +| `sdk-uve` | `dotCMS/src/main/webapp/ext/uve/dot-uve.js` | + +If the source changes and nobody runs `build:js` by hand, the committed file silently drifts out of sync with it, and nothing in CI notices. Out of scope for the strict-mode rollout, but worth a ticket. + +## Commands + +```bash +cd core-web +pnpm exec tsc -p libs/sdk/client/tsconfig.lib.json --noEmit # 0 +pnpm exec tsc -p libs/sdk/client/tsconfig.spec.json --noEmit # 0 +pnpm exec nx run sdk-client:build --skip-nx-cache +pnpm exec nx run sdk-client:lint +pnpm exec nx run sdk-client:test +``` + +> Node 22.22.3 via nvm; `pnpm` is not on `PATH` in this worktree — use `corepack pnpm`. + +## Testing Strategy + +No new tests. `sdk-client` has 15 spec files and its `test` target runs in CI with no skip tag; ts-jest type-checks them against `tsconfig.spec.json`, which inherits the strict flags. Verification for this issue is compilation- and build-based. + +## Boundaries + +**Always:** back the "already done" verdict with reproducible commands in the issue comment. + +**Ask first:** any change to `libs/sdk/client` source — published package, 5 strict consumers, and nothing needs changing. + +**Never:** re-add flags that are already there, or make a cosmetic edit purely to produce a diff. + +--- + +## Success Criteria + +1. `tsc` clean on both `tsconfig.lib.json` and `tsconfig.spec.json`. +2. `nx run sdk-client:build` / `:lint` / `:test` pass on a no-cache run. +3. `git diff origin/main -- core-web/libs/sdk/client` stays **empty** — the deliverable is a verdict, not a diff. +4. #35942 closed with the evidence, its stale ACs corrected first — same treatment as #35939 and #35941. + +## Steps + +1. Correct the stale ACs on #35942 (`typescript-strict-plugin`, `npx tsc-strict`, `// @ts-strict-ignore`). +2. Comment with the evidence table and reproducible commands. +3. Close as completed — criteria met, just not by this rollout. +4. Commit this `spec.md` alongside those for #35939 and #35941. +5. Do **not** add `Closes #35942` to PR #36957 — there is no diff, and linking it would imply that PR resolved it.