Skip to content

refactor(core-web): TS strict mode across utils-testing, utils, dotcms-js, sdk-create-app + webcomponents groundwork - #36957

Open
nicobytes wants to merge 20 commits into
mainfrom
35932-enable-strict-mode
Open

refactor(core-web): TS strict mode across utils-testing, utils, dotcms-js, sdk-create-app + webcomponents groundwork#36957
nicobytes wants to merge 20 commits into
mainfrom
35932-enable-strict-mode

Conversation

@nicobytes

@nicobytes nicobytes commented Aug 7, 2026

Copy link
Copy Markdown
Member

What

Six steps of the strict-mode rollout (epic #35932), plus groundwork on a seventh:

Issue Project Change
#35945 sdk-react Enable the 5 missing flags + fix 14 TS4111 index-signature accesses
#35944 utils-testing Strict was declared but inert — a stale types: ["jasmine"] aborted all type checking
#35940 utils Enable strict + fix the 32 (+17 spec) resulting type errors
#35939 dotcms-js Enable strict + fix the 38 resulting type errors
#35938 sdk-create-app Enable strict + fix the 2 resulting type errors
#35935 sdk-types No code needed — it was already strict. Documents the rollout pattern instead.

All three add the standard six flags to the project's own tsconfig.json, following the pattern established in #36879 (dotcms-models). tsconfig.base.json stays at "strict": false — the rollout never flips it globally.


dotcms-js (#35939)

The largest of the three: 38 errors across 11 files, in a layer-1 core library with 20 dependent projects, including the dotcms-ui admin app. Six of those dependents are already strict, so this library's loose types were leaking uncertainty into projects that had opted into rigour.

Most fixes correct types that were simply wrong, rather than silencing the compiler:

Site Was Reality
Auth.loginAsUser User The code has always passed null when nobody is impersonating, and every consumer already guards with auth.loginAsUser || auth.user. Now User | null.
StringUtils.getLine string Its own JSDoc says "null if it does not exists". Now string | null.
HttpRequestUtils.getQueryStringParam string Same — JSDoc already documented the null case.
RoutingService.getPortletURL string Returns Map.get(). Now string | undefined.
SiteService.switchSiteById Observable<Site> Emits of(null) when no site is found. Now Observable<Site | null>; its one consumer already handled null.
ResponseView.bodyJsonObject DotCMSResponse<T> Assigned from HttpResponse.body, which is nullable. The surrounding try/catch could never throw and has been removed.

LoginService.urls moved from Record<string, string> to inference-typed, which resolves all 8 TS4111 errors at once and gives each endpoint a named property.

Two definite-assignment assertions were used, each with a TODO: LoginService._auth and SiteService.selectedSite are assigned during init but not in the constructor. Modelling them as | undefined is the truthful type, but their public getters (auth, currentSite) are consumed by already-strict projects, so widening them is a public-API change that belongs in its own issue.

No new any, @ts-ignore, or @ts-expect-error anywhere in this PR.

⚠️ dotcms-js has no build target and is tag-excluded from lint and test, so nothing in CI verifies these flags. They document intent; they do not enforce it. This was an explicit scoping decision — no typecheck target or CI gate was added. The six already-strict consumers provide partial, incidental coverage only. Full reasoning in specs/35939-dotcms-js-strict-mode/spec.md, which is included in this PR.

Blast-radius verification

data-access (a strict consumer) went from 106 type errors to 68, with zero new errors introduced — the honest types upstream remove noise downstream. dotcms-ui typechecks clean apart from a pre-existing missing dotcms-webcomponents/loader dist.


utils (#35940)

32 errors across only 3 files, plus 17 more that appeared in the spec files once the flags propagated through tsconfig.spec.json (baseline there was 0). Both are fixed here — leaving the spec errors would have shipped a regression.

The bulk was one constant. EMPTY_FIELD assigned null to 18 members that DotCMSContentTypeField declares non-nullable:

  • Replaced with zero values of the declared types. Verified safe: nothing compares those members to null strictly — consumers use falsy checks such as isNewField's !field.id — so '', 0 and false behave identically at runtime.
  • clazz has no zero value (DotCMSClazz is a union of concrete Java class names), so EMPTY_FIELD and EMPTY_SYSTEM_FIELD are now Omit<DotCMSContentTypeField, 'clazz'>. They are partial templates, not valid fields, and the type now says so. The derived COLUMN_FIELD / ROW_FIELD / TAB_FIELD already supply their own clazz, so they remain complete.

Other fixes:

Site Change
getFieldsWithoutLayout Truthy .filter() did not narrow the optional row.columns. A type predicate clears the TS2532 and both TS2769 without a cast.
ellipsizeText Accepted null/undefined at runtime — its own guard and its tests say so — but declared string/number. Widened to match, with an explicit limit == null check so later comparisons narrow.
fallbackErrorMessages Typed { [key: number]: string }, mirroring the identical declaration already in libs/data-access/.../dot-upload.service.ts.
dot-utils.ts Bracket access for the six DotCMSContentlet index-signature reads in getImageAssetUrl.
dot-asset.service.ts Explicit types for promises and the two fetchAsset params.

The nine as unknown as casts added are all in spec files, on inputs the tests deliberately pass as invalid, matching the idiom those files already used.

⚠️ Same enforcement gap as dotcms-js: utils has no build target and is tag-excluded from lint and test, so nothing in CI verifies these flags. Accepted trade-off, consistent with #35939.

Blast-radius verification

data-access (strict consumer) went from 68 type errors to 36, zero new. utils-testing (strict) unchanged at its 1 pre-existing error — the Omit did not break its EMPTY_SYSTEM_FIELD spread.


sdk-create-app (#35938)

Two errors, both from flags beyond plain strict:

  • src/index.ts:393process.env.DEBUG needs bracket access under noPropertyAccessFromIndexSignature (TS4111). It is the only process.env.* dot access in the project.
  • src/utils/index.ts:41fetchWithRetry tripped noImplicitReturns (TS7030). The loop returns on success and throws on the last attempt, but with retries < 1 the loop never runs and the function fell through returning undefined. Its only caller (isDotcmsRunning, src/index.ts:506) already guarded with if (res && …), so nothing broke in practice — but the signature was lying. Throwing after the loop closes the gap and narrows the return type to Promise<AxiosResponse>.

No build or CI wiring was needed here. The @nx/esbuild:esbuild executor type-checks before bundling (skipTypeCheck defaults to false and is not overridden), and CI already builds this project via nx run-many -t build (build-test in core-web/pom.xml). The same build runs in the SDK release pipeline (cicd_release-sdk.ymlnx run-many --projects='sdk-*'), so the flags are enforced on every release.


sdk-types (#35935)

libs/sdk/types/tsconfig.json has carried strict: true plus the four extra safety flags since the library was created (#31967), and tsc --noEmit passes with zero errors. It is also already enforced: tsconfig.lib.json sets "declaration": true, so @rollup/plugin-typescript sits in the Rollup chain and fails the build on a strict violation.

So no code change was required. What was missing was documentation, added here to core-web/CLAUDE.md:

  • A ## TypeScript Strict Mode section covering the per-project flags, what actually enforces them, and the Vite exception (esbuild skips type checking, which is why the Nx Vite plugin infers a separate typecheck target).
  • Fixes a line that forbade "strict": true in project tsconfigs. It sat under the Jest config guidance but read as a blanket ban, contradicted docs/frontend/TYPESCRIPT_STANDARDS.md, and blocked the epic outright. The restriction now points at tsconfig.spec.json, which is what it meant.


utils-testing (#35944)

The six strict flags were already in tsconfig.json — but completely inert. tsconfig.lib.json declared "types": ["jasmine"], that package is not installed, so tsc emitted TS2688: Cannot find type definition file for 'jasmine' and stopped before semantic checking. The project reported exactly one error regardless of what the code did.

The reference was stale: nothing uses jasmine, two files use jest.*, and @types/jest is installed. Switching to "types": ["jest"] removed the abort and 27 spurious Cannot find name 'jest' errors, leaving 5 real ones:

Site Fix
clean-up-dialog.ts Untyped fixture param → typed structurally as { nativeElement: unknown }, since only that property is touched (no need to pull in Angular's ComponentFixture)
dot-page-state.service.mock.ts _lock: boolean = nullboolean | null
dot-page-tools.mock.ts ×3 Mock entries carried a tags array that DotPageTool does not declare. Verified nothing in the repo reads .tags off a page tool, so the dead field was removed rather than added to the model in dotcms-models

tsc -p libs/utils-testing/tsconfig.lib.json --noEmit now exits 0 with no CLI overrides — the check is real rather than short-circuited.

Verified across consumers of the touched mocks (cleanUpDialog in 7 files, page-tools mock in 3): data-access 751 tests passed, edit-ema-ui 338 passed.


dotcms-webcomponents (#35943) — groundwork only, not closed

Strict is not enabled here. ~250 errors remain across 38 files, and unlike the other projects this one has no skip:build, so Stencil type-checks it on every PR — flipping the flag early turns CI red. What landed is the part that is correct on its own.

The decorator split, which is the load-bearing decision. Stencil declares runtime-injected members without initializers, colliding with strictPropertyInitialization (139 of the original 375 errors). The fix cannot be uniform:

Decorator Count Fix Why
@Event 57 ! Internal; the runtime creates the EventEmitter
@Element 27 ! Internal; the host element
@State 25 ! Internal component state
@Prop 30 ? Public API

Using ! on @Prop made Stencil emit 28 props as required in components.d.ts — breaking for any TS/JSX consumer. With ? the generated API moves required → optional, which is backward compatible. Measured in the generated file, not assumed.

Two traps recorded on the issue

Stencil under-reports. Its build shows ~10 files / ~39 errors per run, not the total. Measured at the same commit: Stencil 39 errors / 10 files vs tsc 250 / 38. Size this work with tsc, not with build output.

--skip-nx-cache does not clear Stencil's cache. Builds can report green against stale .stencil output. This bit me: 0117273504 annotated a prop, passed a "clean" build, and was actually broken — reverted in f22afce383 after verifying twice with .stencil and the Nx cache cleared.

That prop (dot-binary-text-field's value) is genuinely contradictory: handleFilePaste assigns a File, other paths assign strings, and the template feeds it to an <input value> that accepts neither. No annotation describes the current code — the render path has to be fixed first. Left untyped with a TODO(#35943) so it is not re-annotated in isolation.


sdk-react (#35945)

strict: true was already present; the five companion flags were not. Adding them surfaced 14 errors, all TS4111 — dot access on a type carrying an index signature — resolved with bracket notation. Two origins, same fix:

  • 13 from node.attrs, declared Record<string, any> in @dotcms/types. That type is deliberately left alone: block editor attributes really are dynamic, and it lives in a layer-0 project whose consumers would all be affected.
  • 1 from CSS Modules (styles.row in Row.tsx), whose generated type is also a Record<string, string>.

No behaviour change — bracket access compiles to the same property lookup.

The flags here are genuinely enforced, and that was proved rather than assumed. Reverting one access to dot notation fails the build with @rollup/plugin-typescript TS4111, confirming TypeScript sits in the Rollup chain. The project carries no skip: tags, so CI builds, lints and tests it on every PR, and the same build runs in the SDK release pipeline.

One error remains under plain tsc and is expected: Cannot find module 'virtual:sdk-version' in sdk-client — a Vite virtual module that raw tsc cannot resolve but the build can. It predates this change and is unrelated to strict mode. Worth knowing when measuring, or the count reads 15 instead of 14.

Test plan

dotcms-js

  • pnpm exec tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit — 0 errors (from 38)
  • All six already-strict consumers build green: data-access, global-store, portlets-dot-analytics, portlets-dot-analytics-data-access, portlets-dot-locales-portlet, utils-testing
  • data-access typecheck: 106 → 68 errors, zero new
  • dotcms-ui typecheck clean (one pre-existing unrelated error)
  • dotcms-js lint went from 42 to 41 problems (still tag-excluded)

sdk-create-app

  • tsc --noEmit clean on lib and spec
  • nx run sdk-create-app:build / :lint / :test green
  • CLI smoke test: node dist/libs/sdk/create-app/index.js --help works
  • Negative test: reverting the DEBUG fix makes nx run sdk-create-app:build fail with TS4111 — confirming the build gate is real

Both

  • pnpm exec nx format:check --base=origin/main green
  • No new any / @ts-ignore / @ts-expect-error (verified by diff grep)

Note: neither sdk-create-app nor dotcms-js has usable tests. sdk-create-app has zero test files (passWithNoTests: true); dotcms-js has 3 spec files that do not run (skip:test, and tsconfig.spec.json fails on a pre-existing jasmine types error). A green :test means nothing for either — the real verification is compilation.


Correction: a verification false negative (review follow-up)

A review comment caught a real regression this PR introduced, and the reason it slipped through matters for how the numbers above should be read.

libs/utils-testing/tsconfig.lib.json declares "types": ["jasmine"], and that package is not installed. tsc therefore emits TS2688: Cannot find type definition file for 'jasmine' and stops before semantic checking. So tsc -p libs/utils-testing/tsconfig.lib.json --noEmit reports exactly one error no matter what the code does.

The utils section originally reported "utils-testing unchanged at 1 pre-existing error" as evidence of no regression. That measurement proved nothing — nothing was being type-checked. Running the same config with --types node reveals 33 errors, including a genuine TS2741 caused by retyping EMPTY_SYSTEM_FIELD to Omit<DotCMSContentTypeField, 'clazz'>: the mock at dot-content-types.mock.ts:71 spreads it and never supplies clazz.

Fixed by giving the mock clazz: DotCMSClazzes.TEXT; that config is now at 32 errors, all pre-existing and unrelated.

Because the mock has ~103 consumers whose tests do run in CI, the runtime-value change was verified rather than assumed — clazz went null (pre-PR) → absent (this PR) → TEXT:

  • FieldUtil.isRow / isColumn / isTabDivider compare for equality and return false for all three values.
  • There is no !field.clazz or field.clazz === null anywhere in the repo.
  • Test runs: default-value-property 7/7; dot-content-types-edit 545 passed across 48 suites; data-access 751 passed across 79 suites.

The data-access figures reported elsewhere in this PR (106 → 68 for dotcms-js, 68 → 36 for utils) are not affected — that project has no unresolved types entry, so those runs were doing real semantic checking.

core-web/CLAUDE.md now documents this masking behaviour so the next person does not repeat it.

Other two comments

  • sdk-create-app — the throw said "requires at least 1 retry", but retries is the total attempt count (for (i = 0; i < retries; i++)), so retries = 1 is one attempt and zero retries. Reworded to "attempt".
  • CLAUDE.md verify snippet — hard-coded libs/<project>/tsconfig.lib.json, which resolves for neither nested projects (libs/sdk/create-app, which has no tsconfig.lib.json) nor apps (tsconfig.app.json). Replaced with a <projectRoot> placeholder and both caveats.

Notes for reviewers

Three sibling issues in this rollout turned out not to need the work as written, and were resolved separately:

Closes #35945
Closes #35944
Closes #35940
Closes #35939
Closes #35938
Closes #35935

nicobytes and others added 2 commits August 7, 2026 12:12
`sdk-types` needs no code change: `libs/sdk/types/tsconfig.json` has carried
`strict: true` plus the four extra safety flags since the library was created
(#31967), and `tsc -p tsconfig.lib.json --noEmit` passes with zero errors.

It is already enforced too. Because `tsconfig.lib.json` sets
`"declaration": true`, `@rollup/plugin-typescript` sits in the Rollup chain and
reports type diagnostics, so `sdk-types:build` fails on a strict violation —
verified by removing a constructor assignment and watching the build report
TS2564. CI builds every project via the `build-test` execution in
`core-web/pom.xml`, so the gate already runs on each PR. A dedicated
`typecheck` target would be redundant. `lint` does not catch this: ESLint
reports lint rules, not TS diagnostics.

What was actually missing is documentation, so the remaining 42 projects in
epic #35932 have a pattern to follow:

- Add a `## TypeScript Strict Mode` section covering the per-project flags,
  what enforces them, and the Vite exception (esbuild skips type checking,
  which is why the Nx Vite plugin infers a separate `typecheck` target).
- Fix the line that forbade `"strict": true` in project tsconfigs. It sat under
  the Jest config guidance but read as a blanket ban, contradicted
  `docs/frontend/TYPESCRIPT_STANDARDS.md`, and blocked the epic outright. The
  restriction now points at `tsconfig.spec.json`, which is what it meant.

Closes #35935

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Add the standard per-project strict flags to `libs/sdk/create-app/tsconfig.json`,
following the pattern established in #36879 (dotcms-models). `tsconfig.base.json`
is left at `strict: false`.

Two errors surfaced, both from flags beyond plain `strict`:

- `src/index.ts:393` — `process.env.DEBUG` needs bracket access under
  `noPropertyAccessFromIndexSignature` (TS4111). It is the only `process.env.*`
  dot access in the project.
- `src/utils/index.ts:41` — `fetchWithRetry` tripped `noImplicitReturns`
  (TS7030). The loop returns on success and throws on the last attempt, but with
  `retries < 1` the loop never runs and the function fell through returning
  `undefined`. Its only caller already guarded with `if (res && ...)`, so nothing
  broke in practice, but the signature was lying. Throwing after the loop closes
  the gap and narrows the return type.

No build or CI wiring needed. The `@nx/esbuild:esbuild` executor type-checks
before bundling — `skipTypeCheck` defaults to false and is not overridden — and
CI already builds this project via `nx run-many -t build` (`build-test` in
core-web/pom.xml). The same build runs in the SDK release pipeline
(`cicd_release-sdk.yml` → `nx run-many --projects='sdk-*'`), so the flags are
enforced on every release.

Verified: tsc clean on lib and spec; `nx run sdk-create-app:build/lint/test`
green; `nx affected -t build,lint` green; `node dist/libs/sdk/create-app/index.js
--help` still works. Negative test — reverting the DEBUG fix makes
`nx run sdk-create-app:build` fail with TS4111, confirming the gate is real.

Closes #35938

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude finished @nicobytes's task in 1m 39s —— View job


Code Review

Reviewed the diff against origin/main. This is a TS-strict-mode rollout: almost all changes are type-level (widening return types to reflect reality, !/? decorator annotations in webcomponents, bracket access for index signatures). I focused on the handful of sites where runtime behavior could shift.

New Issues

No blocking issues found. The substantive logic changes are behavior-preserving:

  • response-view.ts — removing the dead try/catch and adding ?. guards is safe; HttpResponse.body was already nullable and the old catch could never fire. The new errorsMessages returns '' instead of throwing when the body is null — a strict improvement.
  • dot-utils.ts:ellipsizeTextlimit == null added before limit <= 0 || isNaN(limit); undefined/null produce the same '' result the old code did via isNaN.
  • FieldUtil.ts:EMPTY_FIELDnull → zero-values and Omit<…, 'clazz'> is consistent with the falsy-check consumers; the dot-content-types.mock.ts fix (clazz: DotCMSClazzes.TEXT) correctly repairs the spread that the Omit would otherwise leave incomplete.
  • login.service.ts / site.service.ts — the two ! definite-assignment assertions are documented with TODO(#35939) and keep the public getter types unchanged, avoiding an API break for strict consumers.

Resolved

  • core-web/libs/sdk/create-app/src/utils/index.ts:125 — Copilot's note on the misleading "retry" wording is fixed; the throw now reads "requires at least 1 attempt" and adds a clarifying comment that retries is the total attempt count.
  • core-web/CLAUDE.md — Copilot's note on the hard-coded libs/<project>/tsconfig.lib.json path is fixed; it now uses a <projectRoot> placeholder with explicit caveats for apps (tsconfig.app.json) and projects without a tsconfig.lib.json.

Notes (non-blocking)

  • site.service.mock.ts_currentSite is now Site | undefined while get currentSite(): Site still declares a non-nullable return. This is only internally consistent because dotcms-js has no build/typecheck gate (as the PR itself flags). Not a runtime bug, but the getter type is now technically lying — worth a follow-up when the enforcement gap is closed.

The self-documented enforcement gap for dotcms-js and utils (no build target, tag-excluded from lint/test → nothing in CI verifies these flags) is an accepted, clearly-disclosed trade-off, not a defect in this PR.

Overall: clean. Approve from a correctness standpoint.
· 35932-enable-strict-mode

@github-actions github-actions Bot added Area : Documentation PR changes documentation files Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries labels Aug 7, 2026
@nicobytes nicobytes changed the title 35932 enable strict mode refactor(core-web): enable TS strict mode in sdk-create-app + document the rollout (#35938, #35935) Aug 7, 2026

Copilot AI left a comment

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.

Pull request overview

This PR opts the sdk-create-app library into the workspace’s incremental TypeScript strict-mode rollout (issue #35932), and adjusts docs/runtime code to align with stricter typing and clearer failure modes.

Changes:

  • Enabled strict TypeScript compiler flags for core-web/libs/sdk/create-app via its project tsconfig.json.
  • Updated fetchWithRetry to throw when misconfigured with < 1 attempts to avoid an implicit undefined return path.
  • Updated strict-mode rollout documentation and adjusted DEBUG env access to bracket notation for noPropertyAccessFromIndexSignature.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
core-web/libs/sdk/create-app/tsconfig.json Enables strict compiler options at the project level for the strict-mode rollout.
core-web/libs/sdk/create-app/src/utils/index.ts Adds an explicit throw path for invalid retries values in fetchWithRetry.
core-web/libs/sdk/create-app/src/index.ts Switches DEBUG env access to process.env['DEBUG'] for strict-mode compatibility.
core-web/CLAUDE.md Documents the strict-mode rollout procedure and clarifies portlet tsconfig guidance.

Comment thread core-web/libs/sdk/create-app/src/utils/index.ts
Comment thread core-web/CLAUDE.md Outdated
Add the standard per-project strict flags to `libs/dotcms-js/tsconfig.json`,
following the pattern from #36879 (dotcms-models), and resolve the 38 errors
they surface across 11 files. `tsconfig.base.json` stays at `strict: false`.

Notable type corrections rather than mechanical silencing:

- `Auth.loginAsUser` was typed `User` but the code has always passed `null`
  when nobody is impersonating, and every consumer already guards with
  `auth.loginAsUser || auth.user`. Corrected to `User | null`.
- `StringUtils.getLine` and `HttpRequestUtils.getQueryStringParam` both
  document "null if it does not exist" but were typed `string`. Corrected.
- `RoutingService.getPortletURL` returns `Map.get()`, so `string | undefined`.
- `SiteService.switchSiteById` emits `of(null)` when no site is found, so
  `Observable<Site | null>`. Its one consumer already handles null.
- `ResponseView` now models `HttpResponse.body` as nullable instead of
  assigning `null` into a non-nullable field inside a `try/catch` that could
  never throw. The dead try/catch is removed.
- `LoginService.urls` is typed by inference instead of `Record<string, string>`,
  which keeps dot access valid and gives each endpoint a named property.

Two definite-assignment assertions were used, each with a TODO: `_auth` and
`selectedSite` are assigned during init but not in the constructor. Modelling
them as `| undefined` is the truthful type, but their public getters (`auth`,
`currentSite`) are consumed by already-strict projects, so widening them is a
public-API change that belongs in its own issue.

No new `any`, `@ts-ignore`, or `@ts-expect-error`.

Verified:
- `tsc -p libs/dotcms-js/tsconfig.lib.json --noEmit` — 0 errors
- All six already-strict consumers build green (data-access, global-store,
  portlets-dot-analytics, portlets-dot-analytics-data-access,
  portlets-dot-locales-portlet, utils-testing)
- `data-access` typecheck went from 106 errors to 68, with zero new errors
  introduced — the honest types upstream remove noise downstream
- `dotcms-ui` typecheck clean apart from a pre-existing missing
  `dotcms-webcomponents/loader` dist
- `nx format:check` green; dotcms-js lint went from 42 to 41 problems

Note: this project has no `build` target and is tag-excluded from lint and
test, so nothing in CI verifies these flags. That was an explicit scoping
decision — no `typecheck` target or CI gate was added. See
`specs/35939-dotcms-js-strict-mode/spec.md`.

Closes #35939

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nicobytes nicobytes changed the title refactor(core-web): enable TS strict mode in sdk-create-app + document the rollout (#35938, #35935) refactor(core-web): enable TS strict mode in dotcms-js and sdk-create-app + document the rollout (#35939, #35938, #35935) Aug 7, 2026
nicobytes and others added 3 commits August 7, 2026 14:16
Add the standard per-project strict flags to `libs/utils/tsconfig.json`,
following the pattern from #36879 (dotcms-models), and resolve the 32 errors
they surface across 3 files. `tsconfig.base.json` stays at `strict: false`.

The flags also propagate to `tsconfig.spec.json`, which surfaced 17 further
errors in the spec files (baseline was 0). Those are fixed here too rather
than left as a regression.

Notable changes:

- `EMPTY_FIELD` assigned `null` to 18 members that `DotCMSContentTypeField`
  declares non-nullable. Replaced with zero values of the declared types.
  Nothing compares those members to `null` strictly — consumers use falsy
  checks such as `isNewField`'s `!field.id` — so `''`, `0` and `false` behave
  identically at runtime.
- `clazz` has no zero value (`DotCMSClazz` is a union of concrete Java class
  names), so `EMPTY_FIELD` and `EMPTY_SYSTEM_FIELD` are now typed
  `Omit<DotCMSContentTypeField, 'clazz'>`. They are partial templates, not
  valid fields, and the type now says so. The derived `COLUMN_FIELD`,
  `ROW_FIELD` and `TAB_FIELD` already supply their own `clazz`.
- `getFieldsWithoutLayout` used a truthy `.filter()` that does not narrow the
  optional `row.columns`. Replaced with a type predicate, which clears the
  TS2532 and both TS2769 errors without a cast.
- `ellipsizeText` accepted `null`/`undefined` at runtime — its own guard and
  its tests document that — but declared `string` and `number`. Widened to
  match, with an explicit `limit == null` check so the later comparisons
  narrow.
- `fallbackErrorMessages` typed `{ [key: number]: string }`, mirroring the
  identical declaration already in `libs/data-access/.../dot-upload.service.ts`.
- `dot-utils.ts` uses bracket access for the six `DotCMSContentlet`
  index-signature reads in `getImageAssetUrl`.

No new `any`, `@ts-ignore`, or `@ts-expect-error`. The nine `as unknown as`
casts added are all in spec files, on inputs the tests deliberately pass as
invalid, matching the idiom those files already used.

Verified:
- `tsc -p libs/utils/tsconfig.lib.json --noEmit` — 0 errors (from 32)
- `tsc -p libs/utils/tsconfig.spec.json --noEmit` — 0 errors (from 17)
- `data-access` typecheck went from 68 errors to 36, zero new
- `utils-testing` unchanged at 1 pre-existing error (missing jasmine types)
- `dotcms-ui` typecheck clean apart from a pre-existing missing
  `dotcms-webcomponents/loader` dist
- `nx format:check` green

Note: `utils` has no `build` target and is tag-excluded from lint and test, so
nothing in CI verifies these flags — the same accepted trade-off as #35939.

Closes #35940

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nicobytes nicobytes changed the title refactor(core-web): enable TS strict mode in dotcms-js and sdk-create-app + document the rollout (#35939, #35938, #35935) refactor(core-web): enable TS strict mode in utils, dotcms-js and sdk-create-app + document the rollout Aug 7, 2026
nicobytes and others added 2 commits August 7, 2026 16:20
Addresses three review comments on #36957.

1. `dot-content-types.mock.ts` — real regression, now fixed.

`dotcmsContentTypeFieldBasicMock` spreads `EMPTY_SYSTEM_FIELD`, which #35940
retyped to `Omit<DotCMSContentTypeField, 'clazz'>`, leaving the mock without a
required property (TS2741). It now supplies `clazz: DotCMSClazzes.TEXT`; callers
that care already override it.

Why the original verification missed it: `libs/utils-testing/tsconfig.lib.json`
declares `"types": ["jasmine"]` and that package is not installed, so tsc emits
`TS2688: Cannot find type definition file for 'jasmine'` and stops before
semantic checking. The "1 error before, 1 after" measurement reported in #35940
therefore proved nothing — nothing was being checked. Running with
`--types node` reveals 33 errors, including the TS2741. It is 32 after this fix.

Verified the runtime-value change, since the mock has ~103 consumers whose
tests do run in CI: `clazz` went `null` (pre-PR) → absent (#35940) → `TEXT`.
`FieldUtil.isRow`/`isColumn`/`isTabDivider` compare for equality and return
false for all three, and there is no `!field.clazz` or `=== null` check
anywhere. Test runs: `default-value-property` 7/7, `dot-content-types-edit`
545 passed across 48 suites, `data-access` 751 passed across 79 suites.

2. `sdk-create-app/src/utils/index.ts` — the throw said "requires at least 1
retry", but `retries` is the total attempt count (`for (i = 0; i < retries)`),
so `retries = 1` means one attempt and zero retries. Reworded to "attempt" and
the ambiguity noted in the comment.

3. `core-web/CLAUDE.md` — the verify snippet hard-coded
`libs/<project>/tsconfig.lib.json`, which resolves for neither nested projects
(`libs/sdk/create-app`, which has no `tsconfig.lib.json`) nor apps
(`tsconfig.app.json`). Replaced with a `<projectRoot>` placeholder plus the two
caveats, a reminder that `tsconfig.spec.json` inherits the flags, and a warning
about unresolved `types` entries masking all semantic diagnostics.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`sdk-uve` needs no change for [08/44]. The six strict flags have been in
`libs/sdk/uve/tsconfig.json` since the library was created (`277cbbc8f7`,
#31242, Feb 2025) as a verbatim copy of `sdk-client`'s config, `tsc --noEmit`
is clean on both lib and spec, and there are zero `any`, `@ts-ignore` or
non-null assertions across 4518 lines.

It is also genuinely enforced, which is what separated `sdk-types` from
`dotcms-js` and `utils`. `rollup.config.cjs` sets `compiler: 'babel'`, but that
governs only transpilation — `@nx/rollup`'s `withNx` always inserts a
TypeScript plugin with `check`/`noEmitOnError` tied to `skipTypeCheck`, which
this project does not set. Two of the three type-checking paths run in CI, and
the `build-test` execution in `core-web/pom.xml` has no `<skip>` element, so it
cannot be turned off.

Issue closed as completed with the evidence; not linked to PR #36957 since
there is no diff and that PR did not resolve it.

Also records an incidental finding, left unfixed: `tsconfig.base.json:104`
maps `@dotcms/uve/types` to a file that does not exist, and nothing imports it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`sdk-client` needs no change for [09/44]. The six strict flags are already in
`libs/sdk/client/tsconfig.json`, `tsc --noEmit` is clean on both lib and spec,
and there are zero `any`, `@ts-ignore` or non-null assertions across 9600 lines
of production source.

Enforcement is unambiguous here, unlike the sibling projects that needed an
argument: `rollup.config.cjs` sets `compiler: 'tsc'` against `tsconfig.lib.json`
with no `skipTypeCheck`, so the build compiles with tsc directly against the
strict config. `tags` is empty and the `build-test` execution in
`core-web/pom.xml` has no `<skip>` element, so that build runs on every PR and
gates every SDK release.

Issue closed as completed with the evidence; not linked to PR #36957 since
there is no diff and that PR did not resolve it.

Also records an emerging pattern for the remaining issues: every `libs/sdk/*`
project checked so far is already strict and already enforced — they share a
tsconfig lineage (sdk-uve's config is a verbatim copy of this one) and all build
through Nx executors that type-check. The unfinished work is concentrated in
the non-SDK libraries and the apps.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two factual corrections to the specs for #35941 and #35942, and one
observation, all surfaced by a follow-up review.

1. Published version was wrong. Both specs quoted the version from the
   local `package.json` (`@dotcms/uve` 1.1.1, `@dotcms/client` 1.2.0) as if
   that were what ships. It is not: the SDK release action rewrites the
   version to the dotCMS release tag under ADR-0019 date lockstep. npm
   `latest` for both is 26.8.7-1 (197 and 262 published versions
   respectively). Both specs now say so explicitly, and the corresponding
   GitHub issue comments have been edited.

2. `sdk-client` has 6 dependents, not 4, and 5 of them are strict rather
   than 3. The Nx graph query used for the original count missed
   `sdk-experiments` and `sdk-create-app`. The lone non-strict consumer is
   `portlets-edit-ema-portlet`, which reaches into `@dotcms/client/internal`.

3. New observation, out of scope for the rollout: `build:js` in both
   `sdk-client` and `sdk-uve` emits an artifact that is committed to git
   (`html/js/editor-js/sdk-editor.js` and `ext/uve/dot-uve.js`), but that
   target is invoked by neither `core-web/pom.xml` nor any workflow. If the
   source changes and nobody runs it by hand, the committed file drifts out
   of sync and nothing notices.

The verdicts for both issues are unchanged — both projects remain already
strict-compliant and enforced.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nicobytes and others added 2 commits August 8, 2026 06:27
#35943

Groundwork for #35943. Strict is **not** enabled yet — 276 type errors remain
across 42 files and Stencil type-checks during `build`, so flipping the flag
before they are fixed would turn CI red. This lands the part that is correct on
its own and leaves the build green.

Stencil declares runtime-injected members without initializers, which collides
with `strictPropertyInitialization`. Handled by decorator kind rather than
uniformly, because the choice is not cosmetic:

- `@Event` (57), `@Element` (27), `@State` (25) → definite assignment `!`.
  These are internal; the Stencil runtime assigns them and they do not appear in
  the generated public API.
- `@Prop` (30) → optional `?` instead. Using `!` here made Stencil emit those
  props as **required** in `components.d.ts` — 28 of them — which is a breaking
  change for any TS/JSX consumer. With `?` the generated API moves the other
  way, from required to optional, which is backward compatible.

Also `dot-binary-text-field`'s `value` prop was `= null` with no annotation, so
under strict TS inferred its type as `null` and the generated API narrowed from
`any` to `null`. It is assigned `''` and file URLs at runtime, so it is now
typed `string | null` — still a narrowing from `any`, but an accurate one.

`components.d.ts` and one readme are regenerated build output and are included
so the repo matches what the build produces.

Two things worth recording for whoever finishes this:

- Do **not** put `"ignoreDeprecations": "6.0"` in this tsconfig. Stencil bundles
  TypeScript 5.8.3, which only accepts `"5.0"` and fails the build with
  `Invalid value for '--ignoreDeprecations'`. The repo's tsc is 6.0.3 and needs
  `"6.0"` to see past the deprecated `baseUrl` / `moduleResolution`, so pass it
  on the CLI. Without it, tsc aborts on TS5101/TS5107 before any semantic
  checking and reports a misleading 2 errors.
- Unlike `utils` and `dotcms-js`, this project has no `skip:build`, so the
  Stencil build is a real CI gate. Strict has to reach 0 in one go.

Verified: `nx run dotcms-webcomponents:build` green from a cleared `.stencil`
cache; `dotcms-ui` typechecks with 0 errors; `nx format:check` green; no `!` on
any member without a Stencil decorator (checked by script).

Refs #35943

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…text-field

`5ea14e8cf8` typed `dot-binary-text-field`'s `value` prop as `string | null`.
That was wrong and broke the Stencil build: `handleFilePaste` assigns a `File`
to it (line 105), alongside the strings assigned elsewhere. Corrected to
`string | File | null`.

The error was missed because the verification builds were reading Stencil's
`.stencil` cache. `nx run <project>:build --skip-nx-cache` skips only the Nx
cache, not Stencil's own, so a build can report green against stale output.
Delete `libs/dotcms-webcomponents/.stencil` before trusting a result.

Verified with both caches cleared (`.stencil` removed and `nx reset`):
`nx run dotcms-webcomponents:build` green, `dotcms-ui` typechecks with 0
errors, `nx format:check` green.

Refs #35943

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nary-text-field

Both `5ea14e8cf8` (`string | null`) and `0117273504` (`string | File | null`)
were wrong, and the second broke the Stencil build.

The prop is genuinely contradictory at runtime and `any` was hiding it:
`handleFilePaste` assigns a `File` to it (line 105), other paths assign strings,
and the template passes it straight to an `<input value>`, which accepts
`string | number | string[]` and therefore neither. No annotation describes the
current code correctly — the render path has to be fixed first, which belongs to
the strict-mode work rather than to this groundwork.

Reverted to the original untyped `= null` and left a TODO(#35943) recording the
contradiction so the next person does not re-annotate it and hit the same wall.

Verified green on two consecutive builds with both `.stencil` and the Nx cache
cleared. The earlier green readings that let this through were stale Stencil
cache: `--skip-nx-cache` does not clear `libs/dotcms-webcomponents/.stencil`.

Refs #35943

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`libs/utils-testing/tsconfig.json` has carried all six strict flags (plus
`strictTemplates`) for some time, but they were inert: `tsconfig.lib.json`
declared `"types": ["jasmine"]` and that package is not installed, so tsc
emitted `TS2688: Cannot find type definition file for 'jasmine'` and **stopped
before semantic checking**. The project reported exactly one error no matter
what the code did.

The reference is stale — nothing here uses jasmine, two files use `jest.*`
APIs, and `@types/jest` is installed. Changed to `"types": ["jest"]`, which
both removes the abort and drops 27 spurious `Cannot find name 'jest'` errors,
leaving 5 real ones:

- `clean-up-dialog.ts` — untyped `fixture` param. Typed structurally as
  `{ nativeElement: unknown }` rather than importing Angular's
  `ComponentFixture`, since only that one property is touched.
- `dot-page-state.service.mock.ts` — `_lock: boolean = null`, now
  `boolean | null`.
- `dot-page-tools.mock.ts` — three mock entries carried a `tags` array that
  `DotPageTool` does not declare. Nothing reads `.tags` off a page tool
  anywhere in the repo, so the dead field was removed rather than added to the
  model in `dotcms-models`.

`tsc -p libs/utils-testing/tsconfig.lib.json --noEmit` now exits 0 with no CLI
overrides — the check is real rather than short-circuited.

Verified across the consumers of the touched mocks (`cleanUpDialog` in 7 files,
the page-tools mock in 3): `data-access` 751 passed, `edit-ema-ui` 338 passed,
`dotcms-ui` typechecks with 0 errors, `nx format:check` green.

Note this project still has no build target and is tagged `skip:test` /
`skip:lint`, so nothing in CI runs this check — the same gap recorded for
`dotcms-js` (#35939) and `utils` (#35940).

Closes #35944

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@nicobytes nicobytes changed the title refactor(core-web): enable TS strict mode in utils, dotcms-js and sdk-create-app + document the rollout refactor(core-web): TS strict mode across utils-testing, utils, dotcms-js, sdk-create-app + webcomponents groundwork Aug 10, 2026
`strict: true` was already present; the five companion flags were not. Adding
them surfaced 14 errors, all `TS4111` — dot access on a type carrying an index
signature — resolved with bracket notation.

Two origins, same fix:

- 13 come from `node.attrs`, declared `Record<string, any>` in `@dotcms/types`.
  That type is left alone: block editor attributes really are dynamic, and it
  lives in a layer-0 project whose consumers would all be affected.
- 1 comes from CSS Modules (`styles.row` in `Row.tsx`), whose generated type is
  also a `Record<string, string>`.

No behaviour change — bracket access compiles to the same property lookup.

Unlike `dotcms-js`, `utils` and `utils-testing`, the flags here are genuinely
enforced. Proved rather than assumed: reverting one access to dot notation
fails the build with `@rollup/plugin-typescript TS4111`, so TypeScript is in
the Rollup chain. The project carries no `skip:` tags, so CI builds, lints and
tests it on every PR, and the same build runs in the SDK release pipeline.

One error remains under plain `tsc` and is expected:
`Cannot find module 'virtual:sdk-version'` in `sdk-client`. It is a Vite
virtual module that raw `tsc` cannot resolve but the build can; it predates
this change and is unrelated to strict mode.

Verified: `sdk-react` build, lint and test green; `sdk-experiments` (its only
internal dependent) builds green; `nx format:check` green; no new `any`,
`@ts-ignore` or `@ts-expect-error`.

Closes #35945

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Documentation PR changes documentation files Area : Frontend PR changes Angular/TypeScript frontend code Area : SDK PR changes SDK libraries

Projects

Status: No status

2 participants