feat(command-dev): support frameworks without a port (#4020)

* feat(command-dev): support frameworks without a port

* test(windows): fix race condition

* test(windows): remove snapshot in favor of string matching

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

Author: erezrokah

npm-shrinkwrap.json

@@ -80,7 +80,7 @@
   "dependencies": {
     "@netlify/build": "^26.1.1",
     "@netlify/config": "^17.0.3",
-    "@netlify/framework-info": "^8.0.0",
+    "@netlify/framework-info": "^8.0.1",
     "@netlify/local-functions-proxy": "^1.1.1",
     "@netlify/plugin-edge-handlers": "^3.0.2",
     "@netlify/plugins-list": "^6.2.1",

package.json

@@ -100,6 +100,16 @@ const getDefaultDist = () => {
   return process.cwd()
 }
 
+const getStaticServerPort = async ({ devConfig }) => {
+  const port = await acquirePort({
+    configuredPort: devConfig.staticServerPort,
+    defaultPort: DEFAULT_STATIC_PORT,
+    errorMessage: 'Could not acquire configured static server port',
+  })
+
+  return port
+}
+
 /**
  *
  * @param {object} param0
@@ -132,11 +142,7 @@ const handleStaticServer = async ({ devConfig, options, projectDir }) => {
   const dist = options.dir || devConfig.publish || getDefaultDist()
   log(`${NETLIFYDEVWARN} Running static server from "${path.relative(path.dirname(projectDir), dist)}"`)
 
-  const frameworkPort = await acquirePort({
-    configuredPort: devConfig.staticServerPort,
-    defaultPort: DEFAULT_STATIC_PORT,
-    errorMessage: 'Could not acquire configured static server port',
-  })
+  const frameworkPort = await getStaticServerPort({ devConfig })
   return {
     ...(devConfig.command && { command: devConfig.command }),
     useStaticServer: true,
@@ -156,7 +162,7 @@ const getSettingsFromFramework = (framework) => {
     dev: {
       commands: [command],
       port: frameworkPort,
-      pollingStrategies,
+      pollingStrategies = [],
     },
     name: frameworkName,
     staticAssetsDirectory: staticDir,
@@ -238,24 +244,40 @@ const handleCustomFramework = ({ devConfig }) => {
   }
 }
 
+const mergeSettings = async ({ devConfig, frameworkSettings = {} }) => {
+  const {
+    command: frameworkCommand,
+    frameworkPort: frameworkDetectedPort,
+    dist,
+    framework,
+    env,
+    pollingStrategies = [],
+  } = frameworkSettings
+
+  const command = devConfig.command || frameworkCommand
+  const frameworkPort = devConfig.targetPort || frameworkDetectedPort
+  // if the framework doesn't start a server, we use a static one
+  const useStaticServer = !(command && frameworkPort)
+  return {
+    command,
+    frameworkPort: useStaticServer ? await getStaticServerPort({ devConfig }) : frameworkPort,
+    dist: devConfig.publish || dist || getDefaultDist(),
+    framework,
+    env,
+    pollingStrategies,
+    useStaticServer,
+  }
+}
+
 /**
  * Handles a forced framework and retrieves the settings for it
  * @param {*} param0
  * @returns {Promise<import('./types').BaseServerSettings>}
  */
 const handleForcedFramework = async ({ devConfig, projectDir }) => {
   // this throws if `devConfig.framework` is not a supported framework
-  const { command, dist, env, framework, frameworkPort, pollingStrategies } = getSettingsFromFramework(
-    await getFramework(devConfig.framework, { projectDir }),
-  )
-  return {
-    command: devConfig.command || command,
-    frameworkPort: devConfig.targetPort || frameworkPort,
-    dist: devConfig.publish || dist,
-    framework,
-    env,
-    pollingStrategies,
-  }
+  const frameworkSettings = getSettingsFromFramework(await getFramework(devConfig.framework, { projectDir }))
+  return mergeSettings({ devConfig, frameworkSettings })
 }
 
 /**
@@ -285,15 +307,7 @@ const detectServerSettings = async (devConfig, options, projectDir) => {
       settings = await handleStaticServer({ options, devConfig, projectDir })
     } else {
       validateFrameworkConfig({ devConfig })
-      const { command, frameworkPort, dist, framework, env, pollingStrategies = [] } = frameworkSettings || {}
-      settings = {
-        command: devConfig.command || command,
-        frameworkPort: devConfig.targetPort || frameworkPort,
-        dist: devConfig.publish || dist || getDefaultDist(),
-        framework,
-        env,
-        pollingStrategies,
-      }
+      settings = await mergeSettings({ devConfig, frameworkSettings })
     }
   } else if (devConfig.framework === '#custom') {
     validateFrameworkConfig({ devConfig })

src/utils/detect-server-settings.js

@@ -18,7 +18,7 @@ export type BaseServerSettings = {
   dist: string
 
   // static serving
-  useStaticServer?: true
+  useStaticServer?: boolean
 
   // Framework specific part
   /** A port where a proxy can listen to it */

src/utils/types.d.ts

@@ -301,3 +301,31 @@ test('should pass framework-info env to framework sub process', async (t) => {
     t.snapshot(normalize(error.stdout))
   })
 })
+
+test('should start static service for frameworks without port, forced framework', async (t) => {
+  await withSiteBuilder('site-with-remix', async (builder) => {
+    await builder.withNetlifyToml({ config: { dev: { framework: 'remix' } } }).buildAsync()
+
+    // a failure is expected since this is not a true remix project
+    const error = await t.throwsAsync(() => withDevServer({ cwd: builder.directory }, () => {}, true))
+    t.true(error.stdout.includes(`Failed running command: remix watch. Please verify 'remix' exists`))
+  })
+})
+
+test('should start static service for frameworks without port, detected framework', async (t) => {
+  await withSiteBuilder('site-with-remix', async (builder) => {
+    await builder
+      .withPackageJson({
+        packageJson: {
+          dependencies: { remix: '^1.0.0', '@remix-run/netlify': '^1.0.0' },
+          scripts: {},
+        },
+      })
+      .withContentFile({ path: 'remix.config.js', content: '' })
+      .buildAsync()
+
+    // a failure is expected since this is not a true remix project
+    const error = await t.throwsAsync(() => withDevServer({ cwd: builder.directory }, () => {}, true))
+    t.true(error.stdout.includes(`Failed running command: remix watch. Please verify 'remix' exists`))
+  })
+})

tests/framework-detection.test.js

@@ -32,7 +32,7 @@ const getExecaOptions = ({ cwd, env }) => ({
   encoding: 'utf8',
 })
 
-const startServer = async ({ cwd, offline = true, env = {}, args = [] }) => {
+const startServer = async ({ cwd, offline = true, env = {}, args = [], expectFailure = false }) => {
   const tryPort = currentPort
   currentPort += 1
   const port = await getPort({ port: tryPort })
@@ -49,7 +49,7 @@ const startServer = async ({ cwd, offline = true, env = {}, args = [] }) => {
     let selfKilled = false
     ps.stdout.on('data', (data) => {
       outputBuffer.push(data)
-      if (data.includes('Server now ready on')) {
+      if (!expectFailure && data.includes('Server now ready on')) {
         resolve({
           url,
           host,
@@ -75,7 +75,7 @@ const startDevServer = async (options, expectFailure) => {
   // eslint-disable-next-line fp/no-loops
   for (let attempt = 1; attempt <= maxAttempts; attempt++) {
     try {
-      const { timeout, ...server } = await startServer(options)
+      const { timeout, ...server } = await startServer({ ...options, expectFailure })
       if (timeout) {
         throw new Error(`Timed out starting dev server.\nServer Output:\n${server.output}`)
       }