fix(dev): polish and improve dev command output (#7249)

* fix: fix framework server loading spinner rendering

This is a regression in the loading spinner shown before "Waiting for framework port", introduced in 19.0.3.

* fix: fix framework server spinner

See:
1. #7026
2. #7131
3. #7124

Oops, I was really overthinking this. All this needs to do is, every time a chunk comes in
from the underlying framework dev server, clear the spinner line, write the chunk, then
resume the spinner.

This wasn't set up right at all, which was obfuscated by the erroneous use of `isSpinning`
(see previous commit).

* fix: remove "Netlify Dev" prefix from deploy log

* fix(dev): polish and improve dev command

- Reworded messages for clarity and consistency
  - I think "Netlify Dev" was a vestige of Netlify CLI's predecessor...? In any case, I
    dropped this wording.
  - e.g. Starting Netlify Dev with Astro → Starting Astro dev server
  - e.g. Waiting for framework port 4321 → Waiting for Astro dev server to be ready on port 4321
- Removed the "🔸 Netlify Dev 🔸" first line (doesn't seem to add anything)
- Attempted to address the common confusion around the two printed URLs (our dev server +
  framework server):
  - more distinctive box
  - 'inverted' colour on the URL (cyan background) to make it stand out the most
  - changed `Server now ready on http...` to `Local dev server ready: http...`
- Attempted to address the common confusion around this message: `✔ Waiting for framework
  port 4321. This can be configured using the 'targetPort' property in the netlify.toml`
  - (Users often think this is an error message. I don't blame them.)
  - I changed this to `Waiting for Astro dev server to be ready on port 4321` (where Astro is the framework name)
  - I added logic so that after 5 seconds of waiting the message changes to `Still waiting
    for server on port 4321 to be ready. Are you sure this is the correct port for your
    Astro site? Change this with the targetPort option in your netlify.toml.`.
- Collapse "Injected env var" messages into a single line per source
- ◈ → ⬥

* fix: revert the framework server log prefix for now

Author: serhalp

src/commands/deploy/deploy.ts

@@ -26,7 +26,6 @@ import { BACKGROUND_FUNCTIONS_WARNING } from '../../lib/log.js'
 import { type Spinner, startSpinner, stopSpinner } from '../../lib/spinner.js'
 import { detectFrameworkSettings, getDefaultConfig } from '../../utils/build-info.js'
 import {
-  NETLIFYDEV,
   NETLIFYDEVERR,
   NETLIFYDEVLOG,
   chalk,
@@ -72,7 +71,7 @@ const triggerDeploy = async ({
       })
     } else {
       log(
-        `${NETLIFYDEV} A new deployment was triggered successfully. Visit https://app.netlify.com/sites/${siteData.name}/deploys/${siteBuild.deploy_id} to see the logs.`,
+        `${NETLIFYDEVLOG} A new deployment was triggered successfully. Visit https://app.netlify.com/sites/${siteData.name}/deploys/${siteBuild.deploy_id} to see the logs.`,
       )
     }
   } catch (error_) {

src/commands/dev-exec/dev-exec.ts

@@ -10,7 +10,6 @@ export const devExec = async (cmd: string, options: OptionValues, command: BaseC
 
   // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
   const withEnvelopeEnvVars = await getEnvelopeEnv({ api, context: options.context, env: cachedConfig.env, siteInfo })
-  // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
   const withDotEnvVars = await getDotEnvVariables({ devConfig: { ...config.dev }, env: withEnvelopeEnvVars, site })
 
   injectEnvVariables(withDotEnvVars)

src/commands/dev/dev.ts

@@ -7,9 +7,8 @@ import { OptionValues } from 'commander'
 import { BLOBS_CONTEXT_VARIABLE, encodeBlobsContext, getBlobsContextWithEdgeAccess } from '../../lib/blobs/blobs.js'
 import { promptEditorHelper } from '../../lib/edge-functions/editor-helper.js'
 import { startFunctionsServer } from '../../lib/functions/server.js'
-import { printBanner } from '../../utils/banner.js'
+import { printBanner } from '../../utils/dev-server-banner.js'
 import {
-  NETLIFYDEV,
   NETLIFYDEVERR,
   NETLIFYDEVLOG,
   NETLIFYDEVWARN,
@@ -74,7 +73,6 @@ const handleLiveTunnel = async ({
 }
 
 export const dev = async (options: OptionValues, command: BaseCommand) => {
-  log(NETLIFYDEV)
   const { api, cachedConfig, config, repositoryRoot, site, siteInfo, state } = command.netlify
   config.dev = config.dev != null ? { ...config.dev } : undefined
   config.build = { ...config.build }
@@ -156,7 +154,7 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
   process.env.URL = url
   process.env.DEPLOY_URL = url
 
-  log(`${NETLIFYDEVWARN} Setting up local development server`)
+  log(`${NETLIFYDEVLOG} Setting up local dev server`)
 
   const { configMutations, configPath: configPathOverride } = await runDevTimeline({
     command,
@@ -169,7 +167,6 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
   })
 
   // FIXME(serhalp): `applyMutations` is `(any, any) => any)`. Add types in `@netlify/config`.
-
   const mutatedConfig: typeof config = applyMutations(config, configMutations)
 
   const functionsRegistry = await startFunctionsServer({

src/commands/functions/functions-serve.ts

@@ -4,7 +4,7 @@ import { OptionValues } from 'commander'
 
 import { getBlobsContextWithEdgeAccess } from '../../lib/blobs/blobs.js'
 import { startFunctionsServer } from '../../lib/functions/server.js'
-import { printBanner } from '../../utils/banner.js'
+import { printBanner } from '../../utils/dev-server-banner.js'
 import {
   UNLINKED_SITE_MOCK_ID,
   acquirePort,

src/commands/serve/serve.ts

@@ -10,7 +10,7 @@ import {
 } from '../../lib/blobs/blobs.js'
 import { promptEditorHelper } from '../../lib/edge-functions/editor-helper.js'
 import { startFunctionsServer } from '../../lib/functions/server.js'
-import { printBanner } from '../../utils/banner.js'
+import { printBanner } from '../../utils/dev-server-banner.js'
 import {
   NETLIFYDEVERR,
   NETLIFYDEVLOG,

src/utils/banner.ts

@@ -61,11 +61,13 @@ export const USER_AGENT = `${name}/${version} ${platform}-${arch} node-${process
 const BASE_FLAGS = new Set(['--debug', '--http-proxy', '--http-proxy-certificate-filename'])
 
 export const NETLIFY_CYAN = chalk.rgb(40, 180, 170)
+export const NETLIFY_CYAN_HEX = '#28b5ac'
 
-export const NETLIFYDEV = `${chalk.greenBright('◈')} ${NETLIFY_CYAN('Netlify Dev')} ${chalk.greenBright('◈')}`
-export const NETLIFYDEVLOG = chalk.greenBright('◈')
-export const NETLIFYDEVWARN = chalk.yellowBright('◈')
-export const NETLIFYDEVERR = chalk.redBright('◈')
+// TODO(serhalp) I *think* this "dev" naming is a vestige of the predecessor of the CLI? Rename to avoid
+// confusion with `netlify dev` command?
+export const NETLIFYDEVLOG = chalk.greenBright('⬥')
+export const NETLIFYDEVWARN = chalk.yellowBright('⬥')
+export const NETLIFYDEVERR = chalk.redBright('⬥')
 
 export const BANG = process.platform === 'win32' ? '»' : '›'
 

src/utils/command-helpers.ts

@@ -0,0 +1,18 @@
+import boxen from 'boxen'
+
+import { chalk, log, NETLIFY_CYAN_HEX } from './command-helpers.js'
+
+export const printBanner = (options: { url: string }): void => {
+  log(
+    boxen(`Local dev server ready: ${chalk.inverse.cyan(options.url)}`, {
+      padding: 1,
+      margin: 1,
+      textAlignment: 'center',
+      borderStyle: 'round',
+      borderColor: NETLIFY_CYAN_HEX,
+      // This is an intentional half-width space to work around a unicode padding math bug in boxen
+      title: '⬥ ',
+      titleAlignment: 'center',
+    }),
+  )
+}

src/utils/dev-server-banner.ts

@@ -8,7 +8,7 @@ import { supportsBackgroundFunctions } from '../lib/account.js'
 
 import { NETLIFYDEVLOG, chalk, logAndThrowError, log, warn, APIError } from './command-helpers.js'
 import { loadDotEnvFiles } from './dot-env.js'
-import type { SiteInfo } from './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.
 const ENV_VAR_SOURCES = {
@@ -167,10 +167,9 @@ const getEnvSourceName = (source) => {
 
 /**
  * @param {{devConfig: any, env: Record<string, { sources: string[], value: string}>, site: any}} param0
- * @returns {Promise<Record<string, { sources: string[], value: string}>>}
  */
 // @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 }) => {
+export const getDotEnvVariables = async ({ devConfig, env, site }): Promise<EnvironmentVariables> => {
   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 }) => {
@@ -195,20 +194,15 @@ export const getDotEnvVariables = async ({ devConfig, env, site }) => {
 
 /**
  * Takes a set of environment variables in the format provided by @netlify/config and injects them into `process.env`
- * @param {Record<string, { sources: string[], value: string}>} env
- * @return {void}
  */
-// @ts-expect-error TS(7006) FIXME: Parameter 'env' implicitly has an 'any' type.
-export const injectEnvVariables = (env) => {
+export const injectEnvVariables = (env: EnvironmentVariables): void => {
+  const envVarsToLogByUsedSource: Record<string, string[]> = {}
   for (const [key, variable] of Object.entries(env)) {
     const existsInProcess = process.env[key] !== undefined
-    // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
     const [usedSource, ...overriddenSources] = existsInProcess ? ['process', ...variable.sources] : variable.sources
     const usedSourceName = getEnvSourceName(usedSource)
-    // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
     const isInternal = variable.sources.includes('internal')
 
-    // @ts-expect-error TS(7006) FIXME: Parameter 'source' implicitly has an 'any' type.
     overriddenSources.forEach((source) => {
       const sourceName = getEnvSourceName(source)
 
@@ -224,13 +218,18 @@ export const injectEnvVariables = (env) => {
     if (!existsInProcess || isInternal) {
       // Omitting `general` and `internal` env vars to reduce noise in the logs.
       if (usedSource !== 'general' && !isInternal) {
-        log(`${NETLIFYDEVLOG} Injected ${usedSourceName} env var: ${chalk.yellow(key)}`)
+        envVarsToLogByUsedSource[usedSource] ??= []
+        envVarsToLogByUsedSource[usedSource].push(key)
       }
 
-      // @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
       process.env[key] = variable.value
     }
   }
+
+  for (const [source, keys] of Object.entries(envVarsToLogByUsedSource)) {
+    const sourceName = getEnvSourceName(source)
+    log(`${NETLIFYDEVLOG} Injected ${sourceName} env vars: ${keys.map((key) => chalk.yellow(key)).join(', ')}`)
+  }
 }
 
 export const acquirePort = async ({

src/utils/dev.ts

@@ -4,13 +4,13 @@ import waitPort from 'wait-port'
 
 import { startSpinner, stopSpinner } from '../lib/spinner.js'
 
-import { logAndThrowError, log, NETLIFYDEVERR, NETLIFYDEVLOG } from './command-helpers.js'
+import { logAndThrowError, log, NETLIFYDEVERR, NETLIFYDEVLOG, chalk } from './command-helpers.js'
 import { runCommand } from './shell.js'
 import { startStaticServer } from './static-server.js'
 import type { ServerSettings } from './types.js'
 
-// 10 minutes
-const FRAMEWORK_PORT_TIMEOUT = 6e5
+const FRAMEWORK_PORT_TIMEOUT_MS = 10 * 60 * 1000
+const FRAMEWORK_PORT_WARN_TIMEOUT_MS = 5 * 1000
 
 interface StartReturnObject {
   ipVersion?: 4 | 6
@@ -34,10 +34,11 @@ export const startFrameworkServer = async function ({
     return { ipVersion: family === 'IPv6' ? 6 : 4 }
   }
 
-  log(`${NETLIFYDEVLOG} Starting Netlify Dev with ${settings.framework || 'custom config'}`)
+  log('')
+  log(`${NETLIFYDEVLOG} Starting ${settings.framework || 'framework'} dev server`)
 
   const spinner = startSpinner({
-    text: `Waiting for framework port ${settings.frameworkPort}. This can be configured using the 'targetPort' property in the netlify.toml`,
+    text: `Waiting for ${settings.framework || 'framework'} dev server to be ready on port ${settings.frameworkPort}`,
   })
 
   if (settings.clearPublishDirectory && settings.dist) {
@@ -55,24 +56,45 @@ export const startFrameworkServer = async function ({
       const ipVersion = parseInt(process.versions.node.split('.')[0]) >= 18 ? 6 : 4
       port = { open: true, ipVersion }
     } else {
-      port = await waitPort({
+      const waitPortPromise = waitPort({
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
         port: settings.frameworkPort!,
         host: 'localhost',
         output: 'silent',
-        timeout: FRAMEWORK_PORT_TIMEOUT,
+        timeout: FRAMEWORK_PORT_TIMEOUT_MS,
         ...(settings.pollingStrategies?.includes('HTTP') && { protocol: 'http' }),
       })
 
+      const timerId = setTimeout(() => {
+        if (!port?.open) {
+          spinner.update({
+            text: `Still waiting for server on port ${
+              settings.frameworkPort
+            } to be ready. Are you sure this is the correct port${
+              settings.framework ? ` for ${settings.framework}` : ''
+            }? Change this with the ${chalk.yellow('targetPort')} option in your ${chalk.yellow('netlify.toml')}.`,
+          })
+        }
+      }, FRAMEWORK_PORT_WARN_TIMEOUT_MS)
+
+      port = await waitPortPromise
+      clearTimeout(timerId)
+
       if (!port.open) {
         throw new Error(`Timed out waiting for port '${settings.frameworkPort}' to be open`)
       }
     }
-    spinner.success()
+    spinner.success(`${settings.framework || 'framework'} dev server ready on port ${settings.frameworkPort}`)
   } catch (error_) {
     stopSpinner({ error: true, spinner })
     log(NETLIFYDEVERR, `Netlify Dev could not start or connect to localhost:${settings.frameworkPort}.`)
     log(NETLIFYDEVERR, `Please make sure your framework server is running on port ${settings.frameworkPort}`)
+    log(
+      NETLIFYDEVERR,
+      `If not, you can configure it using the ${chalk.yellow('targetPort')} option in your ${chalk.yellow(
+        'netlify.toml',
+      )}.`,
+    )
     return logAndThrowError(error_)
   }
 

src/utils/framework-server.ts

@@ -36,6 +36,8 @@ const cleanupBeforeExit = async ({ exitCode }: { exitCode?: number | undefined }
   }
 }
 
+// TODO(serhalp): Move (or at least rename). This sounds like a generic shell util but it's specific
+// to `netlify dev`...
 export const runCommand = (
   command: string,
   options: {