From e1372a3c228246c7feb9bf4f4e63dad547b2bfa9 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 8 Jan 2024 11:30:10 +0100 Subject: [PATCH] refactor: type cachedConfig.env (#6288) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: type cachedConfig.env * refactor: rename to less verbose `version` * refactor: use getPort * refactor: replace some deprecated signatures * Update src/commands/types.d.ts Co-authored-by: Eduardo Bouças --------- Co-authored-by: Eduardo Bouças --- src/commands/base-command.ts | 4 +- src/commands/env/env-list.ts | 19 ++--- src/commands/types.d.ts | 7 +- src/utils/command-helpers.ts | 6 +- .../commands/dev/dev.config.test.js | 69 ++++++++++--------- 5 files changed, 58 insertions(+), 47 deletions(-) diff --git a/src/commands/base-command.ts b/src/commands/base-command.ts index c00b51990c0..90559d63e6a 100644 --- a/src/commands/base-command.ts +++ b/src/commands/base-command.ts @@ -26,7 +26,7 @@ import { exit, getToken, log, - netlifyCliVersion, + version, normalizeConfig, padLeft, pollForToken, @@ -548,7 +548,7 @@ export default class BaseCommand extends Command { ...apiUrlOpts, }) const { buildDir, config, configPath, env, repositoryRoot, siteInfo } = cachedConfig - env.NETLIFY_CLI_VERSION = { sources: ['internal'], value: netlifyCliVersion } + env.NETLIFY_CLI_VERSION = { sources: ['internal'], value: version } const normalizedConfig = normalizeConfig(config) const agent = await getAgent({ httpProxy: flags.httpProxy, diff --git a/src/commands/env/env-list.ts b/src/commands/env/env-list.ts index 856e0829a4c..b818d3487a0 100644 --- a/src/commands/env/env-list.ts +++ b/src/commands/env/env-list.ts @@ -8,14 +8,22 @@ import logUpdate from 'log-update' import { chalk, error, log, logJson } from '../../utils/command-helpers.js' import { AVAILABLE_CONTEXTS, getEnvelopeEnv, getHumanReadableScopes } from '../../utils/env/index.js' import BaseCommand from '../base-command.js' +import { EnvironmentVariables } from '../types.js' const MASK_LENGTH = 50 const MASK = '*'.repeat(MASK_LENGTH) -// @ts-expect-error TS(7031) FIXME: Binding element 'environment' implicitly has an 'a... Remove this comment to see the full error message -const getTable = ({ environment, hideValues, scopesColumn }) => { +const getTable = ({ + environment, + hideValues, + scopesColumn, +}: { + environment: EnvironmentVariables + hideValues: boolean + scopesColumn: boolean +}) => { const table = new AsciiTable(`Environment variables`) - const headings = ['Key', 'Value', scopesColumn && 'Scope'].filter(Boolean) + const headings = ['Key', 'Value', scopesColumn && 'Scope'].filter(Boolean) as string[] table.setHeading(...headings) table.addRowMatrix( Object.entries(environment).map(([key, variable]) => @@ -23,10 +31,8 @@ const getTable = ({ environment, hideValues, scopesColumn }) => { // Key key, // Value - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. hideValues ? MASK : variable.value || ' ', // Scope - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. scopesColumn && getHumanReadableScopes(variable.scopes), ].filter(Boolean), ), @@ -62,7 +68,6 @@ export const envList = async (options: OptionValues, command: BaseCommand) => { // filter out general sources environment = Object.fromEntries( Object.entries(environment).filter( - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. ([, variable]) => variable.sources[0] !== 'general' && variable.sources[0] !== 'internal', ), ) @@ -70,7 +75,6 @@ export const envList = async (options: OptionValues, command: BaseCommand) => { // Return json response for piping commands if (options.json) { const envDictionary = Object.fromEntries( - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. Object.entries(environment).map(([key, variable]) => [key, variable.value]), ) logJson(envDictionary) @@ -79,7 +83,6 @@ export const envList = async (options: OptionValues, command: BaseCommand) => { if (options.plain) { const plaintext = Object.entries(environment) - // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. .map(([key, variable]) => `${key}=${variable.value}`) .join('\n') log(plaintext) diff --git a/src/commands/types.d.ts b/src/commands/types.d.ts index f776e22570e..7a7434ce143 100644 --- a/src/commands/types.d.ts +++ b/src/commands/types.d.ts @@ -24,6 +24,11 @@ type PatchedConfig = NetlifyTOML & { } } +type EnvironmentVariableScope = 'builds' | 'functions' | 'runtime' | 'post_processing' +type EnvironmentVariableSource = 'account' | 'addons' | 'configFile' | 'general' | 'internal' | 'ui' + +export type EnvironmentVariables = Record + /** * The netlify object inside each command with the state */ @@ -39,7 +44,7 @@ export type NetlifyOptions = { site: NetlifySite siteInfo: $TSFixMe config: PatchedConfig - cachedConfig: Record + cachedConfig: Record & { env: EnvironmentVariables } globalConfig: $TSFixMe state: StateConfig } diff --git a/src/utils/command-helpers.ts b/src/utils/command-helpers.ts index a53f9846858..c71311bb7a2 100644 --- a/src/utils/command-helpers.ts +++ b/src/utils/command-helpers.ts @@ -48,10 +48,10 @@ export const padLeft = (str, count, filler = ' ') => str.padStart(str.length + c const platform = WSL ? 'wsl' : os.platform() const arch = os.arch() === 'ia32' ? 'x86' : os.arch() -const { name, version } = await getPackageJson() +const { name, version: packageVersion } = await getPackageJson() -export const netlifyCliVersion = version -export const USER_AGENT = `${name}/${netlifyCliVersion} ${platform}-${arch} node-${process.version}` +export const version = packageVersion +export const USER_AGENT = `${name}/${version} ${platform}-${arch} node-${process.version}` /** A list of base command flags that needs to be sorted down on documentation and on help pages */ const BASE_FLAGS = new Set(['--debug', '--httpProxy', '--httpProxyCertificateFilename']) diff --git a/tests/integration/commands/dev/dev.config.test.js b/tests/integration/commands/dev/dev.config.test.js index 768f6938e9c..8463789251e 100644 --- a/tests/integration/commands/dev/dev.config.test.js +++ b/tests/integration/commands/dev/dev.config.test.js @@ -2,6 +2,7 @@ import { version } from 'process' import FormData from 'form-data' +import getPort from 'get-port' import fetch from 'node-fetch' import { gte } from 'semver' import { describe, test } from 'vitest' @@ -11,7 +12,7 @@ import { withSiteBuilder } from '../../utils/site-builder.ts' describe.concurrent('commands/dev/config', () => { test('should use [build.environment] and not [context.production.environment]', async (t) => { - await withSiteBuilder('site-with-build-environment', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -29,7 +30,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -39,7 +40,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should use [context.production.environment] when --context=production', async (t) => { - await withSiteBuilder('site-with-build-environment', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -54,7 +55,7 @@ describe.concurrent('commands/dev/config', () => { handler: async () => ({ statusCode: 200, body: `${process.env.TEST_PRODUCTION_ENVIRONMENT}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory, context: 'production' }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -64,7 +65,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should override .env.development with process env', async (t) => { - await withSiteBuilder('site-with-override', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { functions: { directory: 'functions' } } }) .withEnvFile({ path: '.env.development', env: { TEST: 'FROM_DEV_FILE' } }) @@ -74,7 +75,7 @@ describe.concurrent('commands/dev/config', () => { handler: async () => ({ statusCode: 200, body: `${process.env.TEST}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory, env: { TEST: 'FROM_PROCESS_ENV' } }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -84,7 +85,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should override [build.environment] with process env', async (t) => { - await withSiteBuilder('site-with-build-environment-override', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { build: { environment: { TEST: 'FROM_CONFIG_FILE' } }, functions: { directory: 'functions' } }, @@ -95,7 +96,7 @@ describe.concurrent('commands/dev/config', () => { handler: async () => ({ statusCode: 200, body: `${process.env.TEST}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory, env: { TEST: 'FROM_PROCESS_ENV' } }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -105,7 +106,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should replicate Lambda behaviour for synchronous return values', async (t) => { - await withSiteBuilder('site-replicate-aws-sync-behaviour', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder.withNetlifyToml({ config: { functions: { directory: 'functions' } } }).withFunction({ path: 'env.js', handler: () => ({ @@ -113,7 +114,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`) @@ -126,14 +127,14 @@ describe.concurrent('commands/dev/config', () => { }) test('should override value of the NETLIFY_DEV env variable', async (t) => { - await withSiteBuilder('site-with-netlify-dev-override', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder.withNetlifyToml({ config: { functions: { directory: 'functions' } } }).withFunction({ path: 'env.js', // eslint-disable-next-line n/prefer-global/process handler: async () => ({ statusCode: 200, body: `${process.env.NETLIFY_DEV}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory, env: { NETLIFY_DEV: 'FROM_PROCESS_ENV' } }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -143,7 +144,9 @@ describe.concurrent('commands/dev/config', () => { }) test('should provide CLI version in env var', async (t) => { - await withSiteBuilder('site-with-netlify-version-env-var', async (builder) => { + await withSiteBuilder(t, async (builder) => { + const port = await getPort() + await builder .withContentFile({ content: ` @@ -154,7 +157,7 @@ describe.concurrent('commands/dev/config', () => { NETLIFY_CLI_VERSION: process.env.NETLIFY_CLI_VERSION, })) res.end() - }).listen(1234); + }).listen(${port}); `, path: 'devserver.mjs', }) @@ -163,7 +166,7 @@ describe.concurrent('commands/dev/config', () => { dev: { framework: '#custom', command: 'node devserver.mjs', - targetPort: 1234, + targetPort: port, }, }, }) @@ -178,14 +181,14 @@ describe.concurrent('commands/dev/config', () => { }) test('should set value of the CONTEXT env variable', async (t) => { - await withSiteBuilder('site-with-context-override', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder.withNetlifyToml({ config: { functions: { directory: 'functions' } } }).withFunction({ path: 'env.js', // eslint-disable-next-line n/prefer-global/process handler: async () => ({ statusCode: 200, body: `${process.env.CONTEXT}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -195,14 +198,14 @@ describe.concurrent('commands/dev/config', () => { }) test('should set value of the CONTEXT env variable to the --context flag', async (t) => { - await withSiteBuilder('site-with-context-override', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder.withNetlifyToml({ config: { functions: { directory: 'functions' } } }).withFunction({ path: 'env.js', // eslint-disable-next-line n/prefer-global/process handler: async () => ({ statusCode: 200, body: `${process.env.CONTEXT}` }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory, context: 'deploy-preview' }, async (server) => { const response = await fetch(`${server.url}/.netlify/functions/env`).then((res) => res.text()) @@ -212,7 +215,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should redirect using a wildcard when set in netlify.toml', async (t) => { - await withSiteBuilder('site-with-redirect-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -228,7 +231,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/api/ping`).then((res) => res.text()) @@ -238,7 +241,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should pass undefined body to functions event for GET requests when redirecting', async (t) => { - await withSiteBuilder('site-with-get-echo-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -254,7 +257,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/api/echo?ding=dong`).then((res) => res.json()) @@ -269,7 +272,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should pass body to functions event for POST requests when redirecting', async (t) => { - await withSiteBuilder('site-with-post-echo-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -285,7 +288,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/api/echo?ding=dong`, { @@ -309,7 +312,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should pass body to functions event for POST requests with passthrough edge function', async (t) => { - await withSiteBuilder('site-with-post-echo-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -335,7 +338,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/api/echo?ding=dong`, { @@ -359,7 +362,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should return an empty body for a function with no body when redirecting', async (t) => { - await withSiteBuilder('site-with-no-body-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -374,7 +377,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const response = await fetch(`${server.url}/api/echo?ding=dong`, { @@ -392,7 +395,7 @@ describe.concurrent('commands/dev/config', () => { }) test('should handle multipart form data when redirecting', async (t) => { - await withSiteBuilder('site-with-multi-part-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withNetlifyToml({ config: { @@ -408,7 +411,7 @@ describe.concurrent('commands/dev/config', () => { }), }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const form = new FormData() @@ -435,7 +438,7 @@ describe.concurrent('commands/dev/config', () => { }) test.runIf(gte(version, '18.14.0'))('should support functions with streaming responses', async (t) => { - await withSiteBuilder('site-with-streaming-function', async (builder) => { + await withSiteBuilder(t, async (builder) => { builder .withPackageJson({ packageJson: { dependencies: { '@netlify/functions': 'latest' } } }) .withCommand({ command: ['npm', 'install'] }) @@ -483,7 +486,7 @@ describe.concurrent('commands/dev/config', () => { path: 'netlify/functions/streamer.js', }) - await builder.buildAsync() + await builder.build() await withDevServer({ cwd: builder.directory }, async (server) => { const chunks = []