refactor(@angular/cli): centralize environment variable parsing #30825
Conversation
clydin
force-pushed
the
cli/env-vars-cleanup
branch
from
b0db906 to
8ac9cb6
Compare
Refactors the environment options utility to improve maintainability and clarity. This change introduces a centralized `parseTristate` function to handle all boolean-like parsing from environment variables, removing the previous `isEnabled`, `isDisabled`, and `optional` helpers.
clydin
force-pushed
the
cli/env-vars-cleanup
branch
from
8ac9cb6 to
2a7abfa
Compare
clydin
added
the
action: review
| return false; | ||
| } | ||
|
|
||
| return undefined; | ||
| } |
There was a problem hiding this comment.
Consider: Could we remove the "tristate" concept by just providing a default value like parseEnvBoolean(process.env['FOO'], true /* default */)?
There was a problem hiding this comment.
Some of them don't have constant defaults. The third "not present" state is relevant downstream for them.
| export const ngDebug = isEnabled(process.env['NG_DEBUG']); | ||
| export const forceAutocomplete = optional(process.env['NG_FORCE_AUTOCOMPLETE']); | ||
| /** Disables all analytics reporting when the `NG_CLI_ANALYTICS` environment variable is set to '0' or 'false'. */ | ||
| export const analyticsDisabled = parseTristate(process.env['NG_CLI_ANALYTICS']) === false; |
There was a problem hiding this comment.
Consider: Would it make more sense to frame these in the positive and then fall back to a default value with ?? rather than check equality with a specific boolean? I feel analyticsEnabled = parseTristate(...) ?? true would be more intuitive to me at least.
There was a problem hiding this comment.
This one is more of "force analytics disabled" and analytics being enabled involves further logic. There is potential to further refactor the usages and adjust the naming but i wanted to avoid usage changes where possible here.
clydin
added
action: merge
Refactors the environment options utility to improve maintainability and clarity. This change introduces a centralized
parseTristatefunction to handle all boolean-like parsing from environment variables, removing the previousisEnabled,isDisabled, andoptionalhelpers.