Skip to content

Fix F1ServerApp config/bootstrap review findings - #590

Merged
LarsLaskowski merged 3 commits into
mainfrom
claude/fix-issues-304-243-242-241-tfy3nt
Aug 15, 2026
Merged

Fix F1ServerApp config/bootstrap review findings#590
LarsLaskowski merged 3 commits into
mainfrom
claude/fix-issues-304-243-242-241-tfy3nt

Conversation

@LarsLaskowski

@LarsLaskowski LarsLaskowski commented Aug 15, 2026

Copy link
Copy Markdown
Owner

Pull Request

📖 Description

Fixes a batch of low-severity findings from the repository review affecting
F1ServerApp configuration and bootstrap:

  • environment.ts/environment.prod.ts no longer need @ts-ignore: a new
    src/typings.d.ts declares window.env so the runtime-injected config is
    properly typed.
  • main.ts documents why the dev-mode getBaseUrl uses 4812 (the backend's
    dev port from F1Server.WebApi/Properties/launchSettings.json) instead of
    the frontend's own 4810 dev server port.
  • F1ServerApp.esproj's BuildOutputFolder now matches the actual
    dist/f1-server-app output used by angular.json and the Dockerfile.
  • apk add curl uses --no-cache.
  • The non-deterministic npm install -g npm@latest step is removed; the
    build now uses the npm version shipped with the pinned node:26-alpine
    base image.
  • karma.conf.js no longer references @angular-devkit/build-angular
    (not a dependency, and not needed by the actual @angular/build:karma
    test builder). While verifying the test target actually runs, the
    polyfills option in angular.json also needed to be an array rather
    than a string for that builder's schema; both are fixed together.
  • src/assets/env.js and env.template.js now document their relationship:
    env.js is the local dev default (used by ng serve/ng build and
    index.html), while env.template.js is the source template rendered
    into env.js by docker_entrypoint.sh at container startup.

🎫 Issues

#242 (orphaned AppServerModule) was already resolved by a prior change
that removed app.server.module.ts; no further code change was needed here.

👩‍💻 Reviewer Notes

All changes are configuration/documentation-level; no runtime behavior
changes except the nginx base image now being resolved by digest and
apk add curl no longer caching the package index.

📑 Test Plan

npm run build succeeds and confirms the typings.d.ts change removes the
need for @ts-ignore without introducing type errors. The Angular test
target (ng test) now builds and launches Karma/Chrome successfully after
the polyfills fix; no spec files exist yet, matching the current CI setup.

✅ Checklist

General

  • I have tested my changes.
  • I have read the CONTRIBUTING documentation and followed the project's code style guidelines.

⏭ Next Steps

None.

claude and others added 3 commits August 15, 2026 16:03
Resolves several low-severity review findings in F1ServerApp:
- Type the injected window.env config so environment.ts/prod.ts no
  longer need @ts-ignore (F-317).
- Document the hardcoded dev-mode backend port in main.ts (F-316).
- Align F1ServerApp.esproj's BuildOutputFolder with the actual
  angular.json/Dockerfile output path dist/f1-server-app (F-324).
- Digest-pin the nginx base image and tighten apk add with --no-cache
  (F-326); drop the non-deterministic npm self-upgrade so builds use
  the npm shipped with the pinned node:26-alpine image (F-327).
- Fix karma.conf.js to match the @angular/build:karma test builder,
  which does not use @angular-devkit/build-angular; also fix the
  polyfills option in angular.json, which the same builder requires
  as an array, so the test target is actually valid (F-329).
- Document env.js as the local dev default and env.template.js as the
  container-time source template rendered by docker_entrypoint.sh
  (F-390).

F-328 (orphaned AppServerModule) was already resolved by a prior
change that removed app.server.module.ts.
Removed the fixed digest for the NGINX base image.
@sonarqubecloud

Copy link
Copy Markdown

@LarsLaskowski
LarsLaskowski merged commit 4bedde3 into main Aug 15, 2026
6 checks passed
@LarsLaskowski
LarsLaskowski deleted the claude/fix-issues-304-243-242-241-tfy3nt branch August 15, 2026 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment