-
Notifications
You must be signed in to change notification settings - Fork 198
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
breaking: Remove env variable overrides in npm build
#2100
Conversation
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSIContainer
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference (preview)
|
Awaiting template test results: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2753700&view=results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great write up in the CHANGELOG.md
. Thanks for putting in the effort to provide good instructions for folks looking to upgrade!
Note: This change only introduces compatibility issues with previous
staticwebapp
services that also took advantage of.env
file variables being automatically available atazd deploy
time.Unlike other language implementations, our current
npm
implementation auto-injects environment variables when callingnpm run build
duringazd deploy
. This was done to expediently allow static-frontend files to easily perform environment variable embedding, that allows app configuration to be part of the html/js that runs in the client web browser. This may break static web apps, or apps using older template versions ofazd
. In newer versions of our static web apps, we explicitly perform configuration injection using a predeploy hook.There are multiple reasons why we would remove this:
azd up
behavior is currently different than runningprovision
than runningdeploy
. This is a result ofdeploy
being able to observe the.env
file infrastructure outputs fromprovision
. In theup
flow,package
happens beforeprovision
, and thus the.env
file isn't yet available. This makes it hard to reason about whyazd
works in certain scenarios and not in others.npm run build
differs from runningazd package
.In this change, we also remove
NODE_ENV=production
. This is an unnecessary override that does not matter in practice.npx create-react-app
,npm run build
by default ignoresNODE_ENV
and automatically sets this based on the react-script being ran. See the official docs here.In the future, we would perhaps need to reconsider how
azd up
works for static front-end (specificallyswa
) apps. However, this change is a step in the right direction (removing undefined-ness) as we slowly build the correct features based on user demand.