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

[legacy-framework] Load env variables based on APP_ENV or -e flag #2878

Merged
merged 34 commits into from
Dec 3, 2021

Conversation

beerose
Copy link
Contributor

@beerose beerose commented Oct 25, 2021

Closes: blitz-js/legacy-framework#226

What are the changes and their implications?

Bug Checklist

  • Integration test added (see test docs if needed)

Feature Checklist

kodiakhq[bot]
kodiakhq bot previously approved these changes Oct 25, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Oct 26, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Nov 1, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Nov 1, 2021
@beerose
Copy link
Contributor Author

beerose commented Nov 1, 2021

@flybayer I added the ability to set app env to dev, start, build, export. As for the other commands:

  • new doesn't load anything from .env (or did I miss something)?
  • db is deprectaed
  • prisma — loads env variables inside of the prisma library, so that would need more changes right?
  • generate, codegen, install, routes — doesn't need it?
  • console — from what I see it doesn't load env variables, is that right?

I'll start working on the docs once we have the implementation.

@flybayer
Copy link
Member

flybayer commented Nov 2, 2021

Ok this is looking good, but need a few more changes.

  1. Set APP_ENV if -e present in next cli entrypoint.
  2. Replace every call to dotenv-flow with a call to the next-env package
  3. Remove dotenv-flow dependency

Prisma loads .env but none of the other files. So we previously called dotenv-flow which loaded the other files. Now we are replacing that with next-env.

kodiakhq[bot]
kodiakhq bot previously approved these changes Nov 3, 2021
"name": "@next/env",
"version": "11.1.0",
"version": "0.42.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

update to latest

Copy link
Contributor Author

@beerose beerose left a comment

Choose a reason for hiding this comment

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

  • Add integration tests
  • Test locally the next package
  • Brandon has to do a release with new @blitzjs/env package
  • loadEnvConfig should default to process.cwd()

nextjs/packages/next/package.json Outdated Show resolved Hide resolved
nextjs/packages/next-env/package.json Outdated Show resolved Hide resolved
nextjs/test/package-managers/pnpm/app/package.json Outdated Show resolved Hide resolved
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
kodiakhq[bot]
kodiakhq bot previously approved these changes Dec 2, 2021
@beerose beerose merged commit 4aba0d3 into canary Dec 3, 2021
@beerose beerose deleted the env-variables-loading branch December 3, 2021 15:05
@itsdillon itsdillon changed the title Load env variables based on APP_ENV or -e flag [legacy-framework] Load env variables based on APP_ENV or -e flag Jul 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Environment Variable Loading
3 participants