amplitude / Amplitude-TypeScript

TypeScript Amplitude Analytics SDK
https://amplitude.github.io/Amplitude-TypeScript/
MIT License
126 stars 36 forks source link

Ampli CLI `API_URL` and `APP_ENV` env var conflicts #600

Open aweber1 opened 11 months ago

aweber1 commented 11 months ago

Since the @amplitude/ampli CLI library is not on GitHub (why?), I'm opening this issue here.

The ampli CLI for some reason references environment variables in emitted code and unfortunately at least three of the env vars it references are named too generically: process.env.API_URL, process.env.APP_ENV, process.env.APP_URL. I would highly suggest changing those env vars to be scoped to the ampli library, e.g. AMPLI_CLI_API_URL, AMPLI_CLI_APP_ENV, AMPLI_CLI_APP_URL or similar. OR, better yet, don't ship package code that reads env vars - specifically to avoid issues like this. Instead, always make config injectable.

Version: @amplitude/ampli 1.34.0

aweber1 commented 11 months ago

If you happen to be using patch-package, here is a patch to fix this issue until (if) it is resolved:

file name: @amplitude+ampli+1.34.0.patch

diff --git a/node_modules/@amplitude/ampli/lib/constants.js b/node_modules/@amplitude/ampli/lib/constants.js
index 0d7c7a6..4852c93 100644
--- a/node_modules/@amplitude/ampli/lib/constants.js
+++ b/node_modules/@amplitude/ampli/lib/constants.js
@@ -4,7 +4,7 @@ exports.OAuthRefreshTokenTTLHours = exports.OAuthAccessTokenTTLHours = exports.A
 const { version } = require('../package.json');
 const sentryEnabled = process.env.AMPLI_SENTRY !== 'disabled';
 const sentryDSN = sentryEnabled ? 'https://f2587c0897834285be5dab446d5387db@o13027.ingest.sentry.io/5974462' : undefined;
-const environment = process.env.APP_ENV || 'production';
+const environment = 'production';
 exports.ZONE_SETTINGS = {
     us: {
         apiUrl: 'https://data-api.amplitude.com/graphql',
@@ -20,14 +20,14 @@ exports.ZONE_SETTINGS = {
     },
 };
 exports.APP_SETTINGS = Object.freeze({
-    localDevelopment: process.env.LOCAL_DEVELOPMENT === 'true',
+    localDevelopment: false,
     app: {
         environment,
-        version: process.env.AMPLI_VERSION || version,
+        version,
     },
     ampli: (zone) => ({
-        apiUrl: (process.env.API_URL || exports.ZONE_SETTINGS[zone].apiUrl).replace(/\/+$/, ''),
-        webUrl: (process.env.APP_URL || exports.ZONE_SETTINGS[zone].webUrl).replace(/\/+$/, ''),
+        apiUrl: (exports.ZONE_SETTINGS[zone].apiUrl).replace(/\/+$/, ''),
+        webUrl: (exports.ZONE_SETTINGS[zone].webUrl).replace(/\/+$/, ''),
     }),
     sentry: {
         enabled: sentryEnabled,
qingzhuozhen commented 10 months ago

Hi @aweber1, thanks for reporting this and sharing the fix patch. We will look further into this.