philhawksworth / netlify-plugin-form-submissions

Fetch all submissions made to Netlify Forms in your site and stash them as JSON files before your build runs making the data available to your static site generator at build time.
MIT License
2 stars 1 forks source link

Remove `dotenv` #3

Open ehmicky opened 4 years ago

ehmicky commented 4 years ago

Any process.env modification in a Build plugin is propagated in the next plugins and/or build command.

This means using require('dotenv').config() has an unintended side effects. Should it be removed?

philhawksworth commented 4 years ago

Oh. So are we advised not to use dotenv? This is only used to update process.env variables if a .env file is found, which should only be in a local build and never in the netlify CI/CD.

Without it, how might we want people to test plugins locally in their builds?

Happy to adopt whatever suitable recommendation is made here.

ehmicky commented 4 years ago

The problem is that we support Build plugins updating the build command environment variables by modifying process.env.*. So any modifications of process.env.* is propagated.

So dotenv.parse() should be ok to use, but not dotenv.config() since that modifies process.env.*. Unless that is the explicit purpose of the plugin (e.g. netlify-plugin-dotenv).

const { readFileSync } = require('fs')
const pathExists = require('path-exists')

if (pathExists.sync('.env')) {
  const allEnvs = dotenv.parse(readFileSync('.env', 'utf8'))
  ...
}