Fix F1ServerApp config/bootstrap review findings - #590
Merged
Conversation
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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Pull Request
📖 Description
Fixes a batch of low-severity findings from the repository review affecting
F1ServerAppconfiguration and bootstrap:environment.ts/environment.prod.tsno longer need@ts-ignore: a newsrc/typings.d.tsdeclareswindow.envso the runtime-injected config isproperly typed.
main.tsdocuments why the dev-modegetBaseUrluses4812(the backend'sdev port from
F1Server.WebApi/Properties/launchSettings.json) instead ofthe frontend's own
4810dev server port.F1ServerApp.esproj'sBuildOutputFoldernow matches the actualdist/f1-server-appoutput used byangular.jsonand the Dockerfile.apk add curluses--no-cache.npm install -g npm@lateststep is removed; thebuild now uses the npm version shipped with the pinned
node:26-alpinebase image.
karma.conf.jsno longer references@angular-devkit/build-angular(not a dependency, and not needed by the actual
@angular/build:karmatest builder). While verifying the test target actually runs, the
polyfillsoption inangular.jsonalso needed to be an array ratherthan a string for that builder's schema; both are fixed together.
src/assets/env.jsandenv.template.jsnow document their relationship:env.jsis the local dev default (used byng serve/ng buildandindex.html), whileenv.template.jsis the source template renderedinto
env.jsbydocker_entrypoint.shat container startup.🎫 Issues
#242(orphanedAppServerModule) was already resolved by a prior changethat 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 curlno longer caching the package index.📑 Test Plan
npm run buildsucceeds and confirms thetypings.d.tschange removes theneed for
@ts-ignorewithout introducing type errors. The Angular testtarget (
ng test) now builds and launches Karma/Chrome successfully afterthe
polyfillsfix; no spec files exist yet, matching the current CI setup.✅ Checklist
General
⏭ Next Steps
None.