feat(cli): lint rule for a nullish env default - #723
Open
7nohe wants to merge 1 commit into
Open
Conversation
`process.env.FOO ?? 'default'` falls back only on `undefined`, so a key present
but blank keeps '' and names something that does not exist. Six generated
configs shipped that — a session store called '', a cache store called '', an
SMTP port of 0 from `Number('')` — and the fix for each was found by hand, one
report at a time. `guren/no-nullish-env-default` finds the rest.
Reports a non-empty string or numeric fallback only. `?? ''` behaves the same
under either operator, and a non-literal fallback cannot be judged from syntax,
so neither is reported; that is what keeps the rule at zero false positives
across the 35 sites it found here. A nullish chain reports once, on the operand
holding the literal, and converts whole — `A ?? B ?? 'x'` becomes `A || B ||
'x'`, since mixing the operators is a syntax error.
Enabled in this repo and in the .oxlintrc.json the app templates and
`guren add lint` ship: the defect it was written for lives in scaffold output,
so an app that never reads this repo still gets told.
The one exemption is a mail `from` display name, where an empty value is a
choice; it carries a disable comment saying so.
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.
Why
#714 fixed
process.env.FOO ?? 'default'in six scaffolded configs, each foundby hand, one report at a time.
??falls back only onundefined, so a keypresent but blank keeps
''and names something that does not exist — asession store called
''that failed the boot, a cache store called'', anSMTP port of
0fromNumber('').Nothing stopped the seventh. This adds the rule that does, and applies it.
The rule
guren/no-nullish-env-defaultreportsprocess.env.FOO ?? <non-empty string or number literal>.What it deliberately does not report is what keeps it at zero false
positives across the 35 sites it found here:
?? ''— both operators behave identically, so the operator cannot be the bug?? somePort,?? process.env.OTHER) — not judgeablefrom syntax
||— already the fixA nullish chain reports once, on the operand that holds the literal, and
converts whole:
A ?? B ?? 'x'becomesA || B || 'x', because mixing??and
||without parentheses is a syntax error.Applied
35 sites, across
packages/server,packages/create-app/templates,packages/cli/templates,examples/,scripts/andweb/. The ones thatmatter beyond cosmetics:
FOO=blank, beforelambda/index.tsAWS_LAMBDA_FUNCTION_MEMORY_SIZEparseInt('')→NaN128lambda/index.tsAWS_REGION ?? AWS_DEFAULT_REGION''us-east-1dev-assets.tsprocess.env.NODE_ENVas"""development"InertiaEngine.tsGUREN_INERTIA_ENTRY''HOST(5 templates +commands.ts)''0.0.0.0DATABASE_URL/SQLITE_DATABASE_PATH(12 sites)''pathThe
NODE_ENVsites were checked against the--definerule incommon-pitfalls.md: a define replaces theprocess.env.NODE_ENVmemberexpression, which the surrounding operator does not affect, and none of these
are the pinned MCP / docs-viewer gates.
The one exemption is a mail
fromdisplay name, where an empty value is a realchoice; it carries
oxlint-disable-next-linewith that reason.It ships to apps
Enabled in this repo and in the
.oxlintrc.jsonthat the app templates andguren add lintwrite, because the defect it was written for lives in scaffoldoutput — an app that never reads this repo still gets told.
oxlint-export.test.tsnow proves the rule reaches through the published@guren/cli/oxlintsubpath, againstdist.Verified
lint fails
lint,typecheck,build,audit:starter-template,audit:core-first,audit:docs,audit:template-deps,audit:core-semver— exit 0@guren/cli2329 pass,@guren/server2965 pass,create-guren-app112pass,
test:examples214 pass — 0 failsmoke:starterexit 0