Skip to content

Conversation

@SilentCatD
Copy link

@SilentCatD SilentCatD commented Nov 21, 2024

#120

This will serve some purposes:

  • Share some variables between different environments
  • Add an additional fallback mechanism with the base environment file
  • Override existing environment variable inside the base file with a chosen env

Priority will be as follows:

  • Base env file < Env file + merge with

@java-james java-james requested a review from Copilot August 20, 2025 01:03
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements support for loading variables from a base environment file in addition to the main environment file. This allows sharing common variables across different environments and provides a fallback mechanism where the main environment file can override values from the base file.

  • Adds baseEnvFileName parameter to the load() method for specifying an optional base environment file
  • Refactors file loading logic into a reusable _loadLinesFromFile() helper method
  • Implements variable precedence where base environment variables are loaded first, then overridden by main environment file variables

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/src/dotenv.dart Core implementation of base environment file loading with proper precedence handling
example/pubspec.yaml Adds the new base environment file asset to the example app
example/lib/main.dart Updates example to demonstrate base environment file usage
example/assets/.env Adds override example for base environment variable
example/assets/.base.env Creates sample base environment file with shared variables

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

final linesFromMergeWith = mergeWith.entries
.map((entry) => "${entry.key}=${entry.value}")
.toList();
final allLines = linesFromMergeWith..addAll(linesFromFile);
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The precedence order is incorrect. Base environment variables should be added first, but linesFromMergeWith and linesFromFile are combined before base environment variables are processed. This means mergeWith and main file variables won't properly override base environment variables.

Suggested change
final allLines = linesFromMergeWith..addAll(linesFromFile);
final allLines = linesFromFile..addAll(linesFromMergeWith);

Copilot uses AI. Check for mistakes.
final allLines = linesFromMergeWith..addAll(linesFromFile);
final envEntries = parser.parse(allLines);

_envMap.addAll(baseEnvEntries);
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of adding entries to _envMap is incorrect according to the documented priority. Base environment variables are added after main environment variables, but they should be added first so that main environment variables can override them.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant