Draft
Conversation
Guldem
reviewed
Oct 2, 2024
| /// Prefer using [operator[]] to read values. | ||
| @visibleForTesting | ||
| Map<String, String> get map => _map; | ||
| final DefaultParser _parser; |
| part of './dotenv.dart'; | ||
|
|
||
| import 'package:meta/meta.dart'; | ||
| abstract class Parser { |
There was a problem hiding this comment.
I guess this serve as an interface. So it could be abstract interface class Parser
fstof
reviewed
Oct 18, 2024
| }; | ||
|
|
||
| @visibleForTesting | ||
| Map<String, String> get loadedEnv => _env; |
There was a problem hiding this comment.
Making the loaded environment Map fully available (even if just read only) would help to not have to update any existing types in apps.
Since Platform.environment is a plain old Map<String, String> It would be preferred if we can get access to the raw map
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.
Hello @mockturtl 👋
This is a proposal PR which is by no means final, but a prototype for better understanding of the underlying problems and proposed solutions:
Motivation
dotenvis used in many backend dart projects, and we've had a few support tickets raised at globe.dev regarding env vars loading.includeParentEnvironmentI'm not sure why merging
.envandPlatform.environmentis not a default behavior. Typically .env is used to override production env vars locally for development purposes..envfiles are rarely checked in into version control system and cloud providers/hosting environments have their own secure way to define env vars. Bundling.envis less secure, since it's just a plain text inside a file inside an archive with the code.This PR implements merging
Platform.environmentwith whatever is defined in.envas a default behaviour.Initialization API
Compared to NodeJS's
require('dotenv').config(), current initialization API is a bit more complicated, requires calling both constructor ofDotEnvandload(), which has 2 parementers.This PR implements singleton pattern on
DotEnvclass, similar to how it's done in firebase/flutterfire. Reading env var becomes as easy as:Although I'm not sure when this could be useful, there's still a way to have a custom parser:
Let me know what you think, if you're willing to accept these breaking changes, I'm happy to proceed with this PR, and update tests, docs, and readmes.