refactor(config): share the blank-to-undefined env preprocess - #206
Open
christian-macedo wants to merge 2 commits into
Open
refactor(config): share the blank-to-undefined env preprocess#206christian-macedo wants to merge 2 commits into
christian-macedo wants to merge 2 commits into
Conversation
Member
|
Thanks for the contribution! The root-cause analysis is correct, and centralizing the blank-value normalization in #159 already implements and has approval for the same functional fix. Please reframe this PR as a focused configuration refactor that keeps only the shared helper and its reuse across the existing optional schemas, without duplicating #159’s behavior and regression coverage. That would preserve the distinct maintainability improvement from this contribution. |
The empty-string-to-undefined preprocess was copied inline into four optional env declarations (OptionalUrl, OptionalGithubOrganization, OptionalNonEmpty, and FACILITY_PREVIEW_SURFACE_TOKEN). Extract it into a single `blankToUndefined` helper and reuse it so the behavior has one definition. Pure refactor: the parse result is unchanged, and the existing config tests — including the blank-optional-env regression coverage from theam#159 — continue to pass. No behavior change, so nothing to release on its own.
christian-macedo
force-pushed
the
fix/config-blank-optional-env
branch
from
August 28, 2026 15:31
f5ae5a1 to
ea9730e
Compare
christian-macedo
changed the title
christian-macedo
force-pushed
the
fix/config-blank-optional-env
branch
from
August 31, 2026 06:41
ea9730e to
fcc7b0f
Compare
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
2 participants
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.
Focused refactor of
services/api/src/config.ts. Builds on #159, which fixed the empty-optional-env crash and added the regression coverage — this PR changes no behavior and adds no tests.What this does
After #159, the empty-string→undefined preprocess lambda was copied inline into four optional env declarations:
OptionalUrlOptionalGithubOrganizationOptionalNonEmpty(the sevenVERCEL_*/FACILITY_AWS_CODEBUILD_*/PACKAGE_REGISTRY_TOKENfields)FACILITY_PREVIEW_SURFACE_TOKENThis extracts that idiom into a single
blankToUndefinedhelper and reuses it across all four, so the behavior has one definition instead of four copies.What this deliberately does not do
config.test.ts(34 cases, including those) continues to pass unchanged.Scope
services/api/src/config.ts— +10 / −13, single file.Verification
pnpm --filter @facility/api typecheck— cleanvitest run test/config.test.ts— 34 passed (includes fix(api): treat empty optional env vars as unset #159's blank/whitespace regression cases)biome check services/api/src/config.ts— cleanrefactor:— no release impact on its own, matching the no-behavior-change scope.