Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change: Updated ConfigScope.getOptional to fallback to default value when environment variable is an empty string ('') #43

Merged
merged 2 commits into from
Apr 3, 2023

Conversation

fl-lokalise
Copy link
Contributor

@fl-lokalise fl-lokalise commented Mar 31, 2023

Changes

Problem statement

Given

VAR1=
VAR2=

and performing

var1: configScope.getOptionalInteger('VAR1', 42),
var2: configScope.getOptional('VAR2', 'foo'),

the produced configuration has inconsistent fallback behavior

{
    var1: 42,
    var2: '',
}

Proposed solution

Use logical OR operator || instead of ?? in ConfigScope.getOptional.

Notes

Taking into account that `configScope.getOptional('VAR2', 'foo') currently returns an empty string, this change has the ability to break an application.
Both for applications that rely on this behaviour and the ones that were not aware of this fallback behaviour.

Checklist

  • Apply one of following labels; major, minor, patch or skip-release
  • I've updated the documentation, or no changes were necessary
  • I've updated the tests, or no changes were necessary

@fl-lokalise fl-lokalise added major bug Something isn't working labels Mar 31, 2023
@fl-lokalise fl-lokalise changed the title Env empty string value fix ConfigScope.getOptional doesn't fallback to default value when environment variable is an empty string (''). Mar 31, 2023
@fl-lokalise fl-lokalise changed the title ConfigScope.getOptional doesn't fallback to default value when environment variable is an empty string (''). Breaking change: Updated ConfigScope.getOptional to fallback to default value when environment variable is an empty string ('') Apr 3, 2023
@fl-lokalise fl-lokalise marked this pull request as ready for review April 3, 2023 14:56
@fl-lokalise fl-lokalise merged commit 934e054 into main Apr 3, 2023
@fl-lokalise fl-lokalise deleted the env_empty_value_fix branch April 3, 2023 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants