Repository navigation
Fix/theme icon import - #9
Merged
Merged
Conversation
f29a5d7 moved the email checks into src/lib/email-validation.ts and deleted the FieldValidationState alias from main.ts without exporting it, leaving three ts(2304) errors that keep `astro check` red. Type-only, so no runtime effect; this is the last error blocking a typecheck gate in the image build.
astro build strips types and never typechecks, and publish.yml only builds images, so 5333085's missing themeIcon import shipped as a runtime ReferenceError. astro check reports it as ts(2304). It gates the PR, not the image build: a type-only error must never break a local make all. Runs in node:22-alpine with pnpm 11.5.1, as the web Dockerfile does.
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.
fix(types): export FieldValidationState + astro check on every PR
Base:
develop· Branch:fix/theme-icon-import· Reviewer: @SerjimenContext
fe28db5 (@danielfdez17) already fixed the
themeIconimport ondevelop. This PR doesn't touchit. It adds the type export that was still missing, plus the CI gate that would have caught both.
@Serjimen, for context: the theme and email refactors moved code into
src/lib/but didn'timport everything back into
src/scripts/main.ts:themeIconintotheme-config.tswithout importing it. That was a runtimeReferenceError: themeIcon is not definedin the Layout script, and the landing's Sign inbutton stopped working.
type FieldValidationStatefrom main.ts without exporting it fromemail-validation.ts. That left 3 × ts(2304), type-only with no runtime effect, which keepastro checkred.Nothing caught either one: the web image runs
astro build, which never typechecks, andpublish.ymlonly builds and pushes images.Commits
fix(types): export FieldValidationState from email-validation.EmailValidationResult.statenow uses the exported union, and main.ts imports it.
ci: run astro check on every pull request(.github/workflows/typecheck.yml). It runs innode:22-alpinewith pnpm 11.5.1, the same as the web Dockerfile. This is a PR gate, not abuild step, so a type-only error blocks the merge but never a local
make all.Verification
astro checkreports 4 errors, includingmain.ts:303 Cannot find name 'themeIcon'.astro check0 errors (196 files) ·vitest run39/39 passed ·astro buildCompleteactionlint1.7.12 (Docker): exit 0/api/auth/refresh401. Sign in opens theform, and a real sign-in lands in the osionos shell.
landing pageerror: themeIcon is not defined) andPASSES with the fix.
Not covered
astro checkas a required status check ondevelopandmain(repo settings).