Skip to content
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

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

Merged
merged 9 commits into from
Jan 13, 2022
14 changes: 7 additions & 7 deletions npm-shrinkwrap.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
66 changes: 40 additions & 26 deletions src/utils/detect-server-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -156,7 +162,7 @@ const getSettingsFromFramework = (framework) => {
dev: {
commands: [command],
port: frameworkPort,
pollingStrategies,
pollingStrategies = [],
},
name: frameworkName,
staticAssetsDirectory: staticDir,
Expand Down Expand Up @@ -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 })
}

/**
Expand Down Expand Up @@ -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 })
Expand Down
2 changes: 1 addition & 1 deletion src/utils/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
28 changes: 28 additions & 0 deletions tests/framework-detection.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.snapshot(normalize(error.stdout))
})
})

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.snapshot(normalize(error.stdout))
})
})
18 changes: 18 additions & 0 deletions tests/snapshots/framework-detection.test.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -235,3 +235,21 @@ Generated by [AVA](https://avajs.dev).
␊
yurnalist␊
β—ˆ "npm run develop" exited with code *. Shutting down Netlify Dev server`

## should start static service for frameworks without port, forced framework

> Snapshot 1

`β—ˆ Netlify Dev β—ˆβŠ
␊
β—ˆ Static server listening to 88888␊
β—ˆ Failed running command: remix watch. Please verify 'remix' exists`

## should start static service for frameworks without port, detected framework

> Snapshot 1

`β—ˆ Netlify Dev β—ˆβŠ
␊
β—ˆ Static server listening to 88888␊
β—ˆ Failed running command: remix watch. Please verify 'remix' exists`
Binary file modified tests/snapshots/framework-detection.test.js.snap
Binary file not shown.
6 changes: 3 additions & 3 deletions tests/utils/dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand All @@ -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,
Expand All @@ -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}`)
}
Expand Down