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

Highest priority for system variables #274

Merged

Conversation

tobias-trozowski
Copy link
Contributor

As #180 seems to be inactive for a few month i rebased the work of @AlekseyLeshko

AlekseyLeshko and others added 30 commits August 8, 2019 00:27
* chore: replacing mocha chai with jest

* chore: cleanup
Bumps [webpack](https://github.com/webpack/webpack) from 4.43.0 to 4.44.0.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v4.43.0...v4.44.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [webpack](https://github.com/webpack/webpack) from 4.44.0 to 4.44.1.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v4.44.0...v4.44.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
…le#218)

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.10.4 to 7.11.0.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.11.0/packages/babel-preset-env)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.10.5 to 7.11.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.11.1/packages/babel-core)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [sinon](https://github.com/sinonjs/sinon) from 9.0.2 to 9.0.3.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/master/CHANGELOG.md)
- [Commits](sinonjs/sinon@v9.0.2...v9.0.3)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.11.1 to 7.11.4.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.11.4/packages/babel-core)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [dotenv-defaults](https://github.com/mrsteele/dotenv-defaults) from 2.0.0 to 2.0.1.
- [Release notes](https://github.com/mrsteele/dotenv-defaults/releases)
- [Commits](mrsteele/dotenv-defaults@v2.0.0...v2.0.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
…le#230)

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.11.0 to 7.11.5.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.11.5/packages/babel-preset-env)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
* feat: supporting webpack 5

* feat: dropping support for node v9
BREAKING CHANGE: adding additional env replacements for unreferenced variables for webpack v5
BREAKING CHANGE: dropping support for webpack 5
Bumps [sinon](https://github.com/sinonjs/sinon) from 9.0.3 to 9.2.0.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/master/CHANGELOG.md)
- [Commits](sinonjs/sinon@v9.0.3...v9.2.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [@babel/cli](https://github.com/babel/babel/tree/HEAD/packages/babel-cli) from 7.11.6 to 7.12.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.12.1/packages/babel-cli)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
…le#252)

Bumps [@babel/preset-env](https://github.com/babel/babel/tree/HEAD/packages/babel-preset-env) from 7.11.5 to 7.12.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.12.1/packages/babel-preset-env)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
…#255)

Bumps [@babel/register](https://github.com/babel/babel/tree/HEAD/packages/babel-register) from 7.11.5 to 7.12.1.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.12.1/packages/babel-register)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Bumps [@babel/core](https://github.com/babel/babel/tree/HEAD/packages/babel-core) from 7.11.6 to 7.12.3.
- [Release notes](https://github.com/babel/babel/releases)
- [Changelog](https://github.com/babel/babel/blob/main/CHANGELOG.md)
- [Commits](https://github.com/babel/babel/commits/v7.12.3/packages/babel-core)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
Bumps [standard](https://github.com/standard/standard) from 14.3.4 to 15.0.0.
- [Release notes](https://github.com/standard/standard/releases)
- [Changelog](https://github.com/standard/standard/blob/master/CHANGELOG.md)
- [Commits](standard/standard@v14.3.4...v15.0.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
BREAKING CHANGE: Nothing really breaking but there is a definite limitation

Co-authored-by: Matt Steele <hello@mrsteele.dev>
Bumps [sinon](https://github.com/sinonjs/sinon) from 9.2.0 to 9.2.1.
- [Release notes](https://github.com/sinonjs/sinon/releases)
- [Changelog](https://github.com/sinonjs/sinon/blob/master/CHANGELOG.md)
- [Commits](https://github.com/sinonjs/sinon/commits)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
Bumps [webpack](https://github.com/webpack/webpack) from 4.44.2 to 5.3.2.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v4.44.2...v5.3.2)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
Co-authored-by: Matt Steele <hello@mrsteele.dev>
Bumps [standard](https://github.com/standard/standard) from 15.0.0 to 16.0.1.
- [Release notes](https://github.com/standard/standard/releases)
- [Changelog](https://github.com/standard/standard/blob/master/CHANGELOG.md)
- [Commits](standard/standard@v15.0.0...v16.0.1)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
* feat: resolving missing process.env in v5

* chore: resolving tests

Co-authored-by: Matt Steele <hello@mrsteele.dev>
Bumps [webpack](https://github.com/webpack/webpack) from 5.3.2 to 5.4.0.
- [Release notes](https://github.com/webpack/webpack/releases)
- [Commits](webpack/webpack@v5.3.2...v5.4.0)

Signed-off-by: dependabot-preview[bot] <support@dependabot.com>

Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
Co-authored-by: Matt Steele <mrsteele@users.noreply.github.com>
…for-system-variables

# Conflicts:
#	test/index.test.js
@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #274 (45a48d8) into master (61925bc) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #274      +/-   ##
==========================================
+ Coverage   98.70%   98.74%   +0.03%     
==========================================
  Files           2        2              
  Lines         155      159       +4     
  Branches       38       40       +2     
==========================================
+ Hits          153      157       +4     
  Misses          2        2              
Impacted Files Coverage Δ
src/index.js 98.43% <100.00%> (+0.05%) ⬆️
index.js 98.94% <0.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61925bc...45a48d8. Read the comment docs.

Copy link
Owner

@mrsteele mrsteele left a comment

Choose a reason for hiding this comment

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

One other comment: Do you think this should be classified as a breaking change? It changes the expectation of how this plugin should run and could pose a problem for existing implementations.

@@ -217,6 +217,14 @@ function runTests (Obj, name) {
test('should not allow local variables to override systemvars', () => {
expect(envTest({ path: envSystemvars, systemvars: true })['process.env.PATH2'] !== '""').toEqual(true)
})

test('Should give the highest priority for the system variables', () => {
process.env.TEST = 'production'
Copy link
Owner

Choose a reason for hiding this comment

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

A small nit-picky thing, but can we name this variable something more outlandish? TEST is pretty generic.

I'll let you be creative as it is your PR, but if you are looking for inspiration you can name it MATT_MADE_ME_DO_THIS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as far as i can tell this is on purpose as we want to override the defined default variables.
Renaming this would result in renaming all env variables TEST

@tobias-trozowski
Copy link
Contributor Author

Regarding the breaking change question...
This is a fix to a feature which did not work yet.
My expectation of this plugin was that it does, what the documentation already pointed out, system env vars must override any other variable.
On the other side this will surely break things for people who did not read the documentation.

I would not consider this as a breaking change but rather as bugfix to an non-working feature.

@mrsteele
Copy link
Owner

@tobias-trozowski yeah I'm thinking about those people who have it implemented with the idea that system variables will be overwritten. I don't think it would be courteous to just break all of their builds.

@tobias-trozowski
Copy link
Contributor Author

tobias-trozowski commented Nov 21, 2020

This decision is up to you.
It will break for everyone who is using safe: true, systemvars: true. So pushing a new major version would be the most gentle variant.

@mrsteele
Copy link
Owner

Yeah let's do that.

@mrsteele mrsteele merged commit ce5958f into mrsteele:master Nov 25, 2020
@mrsteele
Copy link
Owner

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants