-
Notifications
You must be signed in to change notification settings - Fork 412
refactor: cherry-pick type fixes from #7058 and #7068 #7383
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,9 @@ | |
|
||
import { NETLIFYDEVLOG, chalk, logAndThrowError, log, warn, APIError } from './command-helpers.js' | ||
import { loadDotEnvFiles } from './dot-env.js' | ||
import type { CachedConfig } from '../lib/build.js' | ||
import type { NetlifySite } from '../commands/types.js' | ||
import type { DevConfig } from '../commands/dev/types.js' | ||
import type { EnvironmentVariables, SiteInfo } from './types.js' | ||
|
||
// Possible sources of environment variables. For the purpose of printing log messages only. Order does not matter. | ||
|
@@ -41,8 +44,7 @@ | |
const ERROR_CALL_TO_ACTION = | ||
"Double-check your login status with 'netlify status' or contact support with details of your error." | ||
|
||
// @ts-expect-error TS(7031) FIXME: Binding element 'site' implicitly has an 'any' typ... Remove this comment to see the full error message | ||
const validateSiteInfo = ({ site, siteInfo }) => { | ||
const validateSiteInfo = ({ site, siteInfo }: { site: NetlifySite; siteInfo: SiteInfo }): void => { | ||
if (isEmpty(siteInfo)) { | ||
return logAndThrowError( | ||
`Failed to retrieve project information for project ${chalk.yellow(site.id)}. ${ERROR_CALL_TO_ACTION}`, | ||
|
@@ -73,9 +75,9 @@ | |
} | ||
} | ||
|
||
// @ts-expect-error TS(7031) FIXME: Binding element 'api' implicitly has an 'any' type... Remove this comment to see the full error message | ||
const getAddons = async ({ api, site }) => { | ||
const getAddons = async ({ api, site }: { api: NetlifyAPI; site: NetlifySite }) => { | ||
try { | ||
// @ts-expect-error(serhalp) One of three types is incorrect here (is `site.id` optional?). Dig and fix. | ||
const addons = await api.listServiceInstancesForSite({ siteId: site.id }) | ||
return addons | ||
} catch (error_) { | ||
|
@@ -87,13 +89,11 @@ | |
} | ||
} | ||
|
||
// @ts-expect-error TS(7031) FIXME: Binding element 'addons' implicitly has an 'any' t... Remove this comment to see the full error message | ||
const getAddonsInformation = ({ addons, siteInfo }) => { | ||
type Addons = Awaited<ReturnType<NetlifyAPI['listServiceInstancesForSite']>> | ||
const getAddonsInformation = ({ addons, siteInfo }: { addons: Addons; siteInfo: SiteInfo }) => { | ||
const urls = Object.fromEntries( | ||
// @ts-expect-error TS(7006) FIXME: Parameter 'addon' implicitly has an 'any' type. | ||
addons.map((addon) => [addon.service_slug, `${siteInfo.ssl_url}${addon.service_path}`]), | ||
) | ||
// @ts-expect-error TS(7006) FIXME: Parameter 'addon' implicitly has an 'any' type. | ||
const env = Object.assign({}, ...addons.map((addon) => addon.env)) | ||
return { urls, env } | ||
} | ||
|
@@ -113,17 +113,17 @@ | |
// default 15 minutes for background functions | ||
const BACKGROUND_FUNCTION_TIMEOUT = 900 | ||
|
||
/** | ||
* | ||
* @param {object} config | ||
* @param {boolean} config.offline | ||
* @param {*} config.api | ||
* @param {*} config.site | ||
* @param {*} config.siteInfo | ||
* @returns | ||
*/ | ||
// @ts-expect-error TS(7031) FIXME: Binding element 'api' implicitly has an 'any' type... Remove this comment to see the full error message | ||
export const getSiteInformation = async ({ api, offline, site, siteInfo }) => { | ||
export const getSiteInformation = async ({ | ||
api, | ||
offline, | ||
site, | ||
siteInfo, | ||
}: { | ||
api: NetlifyAPI | ||
offline: boolean | ||
site: NetlifySite | ||
siteInfo: SiteInfo | ||
}) => { | ||
if (site.id && !offline) { | ||
validateSiteInfo({ site, siteInfo }) | ||
const [accounts, addons] = await Promise.all([getAccounts({ api }), getAddons({ api, site })]) | ||
|
@@ -139,7 +139,7 @@ | |
backgroundFunctions: supportsBackgroundFunctions(account), | ||
}, | ||
timeouts: { | ||
syncFunctions: siteInfo.functions_timeout ?? siteInfo.functions_config?.timeout ?? SYNCHRONOUS_FUNCTION_TIMEOUT, | ||
Check failure on line 142 in src/utils/dev.ts
|
||
backgroundFunctions: BACKGROUND_FUNCTION_TIMEOUT, | ||
}, | ||
} | ||
|
@@ -157,21 +157,22 @@ | |
} | ||
} | ||
|
||
// @ts-expect-error TS(7006) FIXME: Parameter 'source' implicitly has an 'any' type. | ||
const getEnvSourceName = (source) => { | ||
// @ts-expect-error TS(7053) FIXME: Element implicitly has an 'any' type because expre... Remove this comment to see the full error message | ||
const { name = source, printFn = chalk.green } = ENV_VAR_SOURCES[source] || {} | ||
const getEnvSourceName = (source: string) => { | ||
const { name = source, printFn = chalk.green } = ENV_VAR_SOURCES[source] ?? {} | ||
Check failure on line 161 in src/utils/dev.ts
|
||
Comment on lines
+160
to
+161
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lots of type errors here. I think an enum or a union of literal strings is needed. |
||
|
||
return printFn(name) | ||
} | ||
|
||
/** | ||
* @param {{devConfig: any, env: Record<string, { sources: string[], value: string}>, site: any}} param0 | ||
*/ | ||
// @ts-expect-error TS(7031) FIXME: Binding element 'devConfig' implicitly has an 'any... Remove this comment to see the full error message | ||
export const getDotEnvVariables = async ({ devConfig, env, site }): Promise<EnvironmentVariables> => { | ||
export const getDotEnvVariables = async ({ | ||
devConfig, | ||
env, | ||
site, | ||
}: { | ||
devConfig: DevConfig | ||
env: CachedConfig['env'] | ||
site: NetlifySite | ||
}): Promise<Record<string, { sources: string[]; value: string }>> => { | ||
const dotEnvFiles = await loadDotEnvFiles({ envFiles: devConfig.envFiles, projectDir: site.root }) | ||
// @ts-expect-error TS(2339) FIXME: Property 'env' does not exist on type '{ warning: ... Remove this comment to see the full error message | ||
dotEnvFiles.forEach(({ env: fileEnv, file }) => { | ||
const newSourceName = `${file} file` | ||
|
||
|
@@ -183,6 +184,7 @@ | |
} | ||
|
||
env[key] = { | ||
// @ts-expect-error(serhalp) Something isn't right with these types but it's a can of worms. | ||
sources, | ||
value: fileEnv[key], | ||
} | ||
|
@@ -248,8 +250,7 @@ | |
return acquiredPort | ||
} | ||
|
||
// @ts-expect-error TS(7006) FIXME: Parameter 'fn' implicitly has an 'any' type. | ||
export const processOnExit = (fn) => { | ||
export const processOnExit = (fn: (...args: unknown[]) => void) => { | ||
const signals = ['SIGINT', 'SIGTERM', 'SIGQUIT', 'SIGHUP', 'exit'] | ||
signals.forEach((signal) => { | ||
process.on(signal, fn) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,44 +1,58 @@ | ||
import { readFile } from 'fs/promises' | ||
import path from 'path' | ||
|
||
import dotenv from 'dotenv' | ||
import dotenv, { type DotenvParseOutput } from 'dotenv' | ||
|
||
import { isFileAsync } from '../lib/fs.js' | ||
|
||
import { warn } from './command-helpers.js' | ||
|
||
// @ts-expect-error TS(7031) FIXME: Binding element 'envFiles' implicitly has an 'any'... Remove this comment to see the full error message | ||
export const loadDotEnvFiles = async function ({ envFiles, projectDir }) { | ||
const response = await tryLoadDotEnvFiles({ projectDir, dotenvFiles: envFiles }) | ||
interface LoadedDotEnvFile { | ||
file: string | ||
env: DotenvParseOutput | ||
} | ||
|
||
export const loadDotEnvFiles = async function ({ | ||
envFiles, | ||
projectDir, | ||
}: { | ||
envFiles?: string[] | ||
projectDir?: string | ||
}): Promise<LoadedDotEnvFile[]> { | ||
const loadedDotEnvFiles = await tryLoadDotEnvFiles({ projectDir, dotenvFiles: envFiles }) | ||
|
||
// @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'. | ||
const filesWithWarning = response.filter((el) => el.warning) | ||
filesWithWarning.forEach((el) => { | ||
// @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'. | ||
warn(el.warning) | ||
}) | ||
loadedDotEnvFiles | ||
.filter((el): el is { warning: string } => 'warning' in el) | ||
.forEach((el) => { | ||
warn(el.warning) | ||
}) | ||
|
||
// @ts-expect-error TS(2532) FIXME: Object is possibly 'undefined'. | ||
return response.filter((el) => el.file && el.env) | ||
return loadedDotEnvFiles.filter((el): el is LoadedDotEnvFile => 'file' in el && 'env' in el) | ||
Comment on lines
-22
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's avoid behavioural changes here, so also check for truthiness |
||
} | ||
|
||
// in the user configuration, the order is highest to lowest | ||
const defaultEnvFiles = ['.env.development.local', '.env.local', '.env.development', '.env'] | ||
|
||
// @ts-expect-error TS(7031) FIXME: Binding element 'projectDir' implicitly has an 'an... Remove this comment to see the full error message | ||
export const tryLoadDotEnvFiles = async ({ dotenvFiles = defaultEnvFiles, projectDir }) => { | ||
export const tryLoadDotEnvFiles = async ({ | ||
dotenvFiles = defaultEnvFiles, | ||
projectDir, | ||
}: { | ||
dotenvFiles?: string[] | ||
projectDir?: string | ||
}): Promise<Array<LoadedDotEnvFile | { warning: string }>> => { | ||
const results = await Promise.all( | ||
dotenvFiles.map(async (file) => { | ||
const filepath = path.resolve(projectDir, file) | ||
const filepath = path.resolve(projectDir ?? '', file) | ||
try { | ||
const isFile = await isFileAsync(filepath) | ||
if (!isFile) { | ||
return | ||
} | ||
} catch (error) { | ||
return { | ||
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'. | ||
warning: `Failed reading env variables from file: ${filepath}: ${error.message}`, | ||
warning: `Failed reading env variables from file: ${filepath}: ${ | ||
error instanceof Error ? error.message : error?.toString() | ||
}`, | ||
} | ||
} | ||
const content = await readFile(filepath, 'utf-8') | ||
|
@@ -48,5 +62,5 @@ | |
) | ||
|
||
// we return in order of lowest to highest priority | ||
return results.filter(Boolean).reverse() | ||
return results.filter((result): result is LoadedDotEnvFile => result != null).reverse() | ||
} |
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.
this introduced several type errors. you may need to fix
SiteInfo
itself?