Drop direct dependencies for config and dotenv

[thecodedrift]

Description
Currently, when we have NODE_ENV=production set, attempting to run node-pg-migrate will throw warnings with no way to silence them.

Reproduction

$> NODE_ENV=production node-pg-migrate up
WARNING: NODE_ENV value of 'production' did not match any deployment config file names.
WARNING: See https://github.com/lorenwest/node-config/wiki/Strict-Mode

Additional Context
The always-throwing-warning was introduced with 1.9.0 of config as part of their Strict Mode release. While you can tell node-config to warn instead of throw, the errors cannot be shut off, even when you are not using config in your app. This happens because we have a direct dependency on config (and dotenv) meaning that we'll always require and run them even if our consuming code doesn't use them.

As a proposed fix, I suggest we leave our require + try/catch intact, and just move config and dotenv into peerDependencies just like we do with pg.

💥 This will introduce a breaking change A change to peerDependencies means that anyone who was assuming a .env or a config/default-<environment>.json file would "just work" will have migrations that do not run without adding the peer dependency.

Workaround
In the meantime, if you're in a CI environment and don't require NODE_ENV for the migration operation, this comment indicates that you can explicitly remove the NODE_ENV value by running NODE_ENV= node-pg-migrate up. It's not an ideal workaround, but it is something in case you're running into weird CI issues.

References

• https://github.com/salsita/node-pg-migrate/blob/master/bin/node-pg-migrate#L237 - example of the existing try/catch
• https://github.com/lorenwest/node-config/wiki/Strict-Mode - strict mode docs stating a warning will always be issued
• how can i suppress "NODE_ENV value of 'test' did not match any deployment config file names." warnings? node-config/node-config#386 - stating that they refuse to fix this and prefer folks "Just create the file."

[Shinigami92]

But they are already optionalDependencies?

node-pg-migrate/package.json

Lines 83 to 86 in e883e5b

	"optionalDependencies": {
	"config": ">=1.0.0",
	"dotenv": ">=1.0.0"
	},

[thecodedrift]

Unfortunately, optionalDependencies still install, they just don't fail thenpm install. https://docs.npmjs.com/files/package.json#optionaldependencies (think chokidar's fsevents as an example)

As a result, pg-node-migrate installs both dotenv and config. 😕

It's pretty poorly named.

[Shinigami92]

So maybe we could move both to peerDependencies and throw a warning if needed?

[thecodedrift]

That would be my recommendation, to move them into peerDependencies. The code is already try/catching the requires responsibily.

The only reason it's a breaking change is someone could be depending on .env files or the config/ directory and not realize it was node-pg-migrate adding things to process.env. It's an undocumented "feature" we'd be removing.

[dolezel]

Problem is, that dotenv and config are really optional dependencies, it is not necessary to use them, bad is that npm installs them regardless. Moving them to peerDependencies would result in a warning during installation for people who do not use them.
There is no way how to tell that dependency is optional and it is up to the user to have it installed if he wants to use it.
The correct solution would be probably to remove them from package.json completely.

[thecodedrift]

@dolezel I'd be okay with dropping them completely. The try/catch does its job properly. Happy to put together a PR for it.

What are your thoughts everyone RE that being a breaking change?

1. 👀 It's a breaking change, because anyone who didn't install their own version of dotenv or config but had a .env or config/ will suddenly lose access to DATABASE_URL
2. 🚀 It's a patch change, because as far as the code is concerned, because end-developers were never intended to consumenode-pg-migrate's copies of these dependencies.