Skip to content

Commit

Permalink
feat: report differences for new framework detection (#5643)
Browse files Browse the repository at this point in the history
* feat: report differences for new framework detection

* chore: pr feedback

* chore: pr feedback from japheth
  • Loading branch information
Lukas Holzer authored Apr 20, 2023
1 parent 5fd6905 commit add7a6d
Show file tree
Hide file tree
Showing 7 changed files with 259 additions and 20 deletions.
13 changes: 11 additions & 2 deletions functions/error-reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const handler: Handler = async ({ body }) => {
cause,
cliVersion,
message,
metadata = {},
name,
nodejsVersion,
osName,
Expand All @@ -24,13 +25,21 @@ export const handler: Handler = async ({ body }) => {
user,
} = JSON.parse(body)
Bugsnag.notify({ name, message, stack, cause }, (event) => {
event.app = {
releaseStage: 'production',
version: cliVersion,
type: 'netlify-cli',
}
// eslint-disable-next-line fp/no-loops
for (const [section, values] of Object.entries(metadata)) {
event.addMetadata(section, values as Record<string, any>)
}
event.setUser(user.id, user.email, user.name)
event.severity = severity
event.device = {
osName,
runtimeVersions: {
nodejs: nodejsVersion,
cli: cliVersion,
node: nodejsVersion,
},
}
})
Expand Down
134 changes: 134 additions & 0 deletions package-lock.json

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

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
"@bugsnag/js": "^7.20.0",
"@fastify/static": "^6.6.0",
"@netlify/build": "^29.9.2",
"@netlify/build-info": "^7.0.0-pre-20230418.0",
"@netlify/config": "^20.3.7",
"@netlify/edge-bundler": "^8.13.2",
"@netlify/framework-info": "^9.8.5",
Expand Down
7 changes: 6 additions & 1 deletion src/commands/dev/dev.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,12 @@ const dev = async (options, command) => {
/** @type {Partial<import('../../utils/types').ServerSettings>} */
let settings = {}
try {
settings = await detectServerSettings(devConfig, options, site.root)
settings = await detectServerSettings(devConfig, options, site.root, {
site: {
id: site.id,
url: siteUrl,
},
})

cachedConfig.config = getConfigWithPlugins(cachedConfig.config, settings)
} catch (error_) {
Expand Down
105 changes: 94 additions & 11 deletions src/utils/detect-server-settings.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ import { EOL } from 'os'
import path from 'path'
import process from 'process'

import { Project } from '@netlify/build-info'
// eslint-disable-next-line import/extensions, n/no-missing-import
import { NodeFS } from '@netlify/build-info/node'
import { getFramework, listFrameworks } from '@netlify/framework-info'
import fuzzy from 'fuzzy'
import getPort from 'get-port'
Expand All @@ -12,6 +15,7 @@ import isPlainObject from 'is-plain-obj'
import { NETLIFYDEVWARN, chalk, log } from './command-helpers.mjs'
import { acquirePort } from './dev.mjs'
import { getInternalFunctionsDir } from './functions/functions.mjs'
import { reportError } from './telemetry/report-error.mjs'

const formatProperty = (str) => chalk.magenta(`'${str}'`)
const formatValue = (str) => chalk.green(`'${str}'`)
Expand Down Expand Up @@ -112,10 +116,10 @@ const getStaticServerPort = async ({ devConfig }) => {
/**
*
* @param {object} param0
* @param {import('../commands/dev/types').DevConfig} param0.devConfig
* @param {import('../commands/dev/types.js').DevConfig} param0.devConfig
* @param {import('commander').OptionValues} param0.options
* @param {string} param0.projectDir
* @returns {Promise<import('./types').BaseServerSettings>}
* @returns {Promise<import('./types.js').BaseServerSettings>}
*/
const handleStaticServer = async ({ devConfig, options, projectDir }) => {
validateNumberProperty({ devConfig, property: 'staticServerPort' })
Expand Down Expand Up @@ -152,8 +156,8 @@ const handleStaticServer = async ({ devConfig, options, projectDir }) => {

/**
* Retrieves the settings from a framework
* @param {import('./types').FrameworkInfo} framework
* @returns {import('./types').BaseServerSettings}
* @param {import('./types.js').FrameworkInfo} framework
* @returns {import('./types.js').BaseServerSettings}
*/
const getSettingsFromFramework = (framework) => {
const {
Expand Down Expand Up @@ -182,6 +186,71 @@ const getSettingsFromFramework = (framework) => {

const hasDevCommand = (framework) => Array.isArray(framework.dev.commands) && framework.dev.commands.length !== 0

/**
* The new build setting detection with build systems and frameworks combined
* @param {string} projectDir
*/
const detectSettings = async (projectDir) => {
const fs = new NodeFS()
const project = new Project(fs, projectDir)

return await project.getBuildSettings()
}

/**
*
* @param {import('./types.js').BaseServerSettings | undefined} frameworkSettings
* @param {import('@netlify/build-info').Settings[]} newSettings
* @param {Record<string, Record<string, any>>} [metadata]
*/
const detectChangesInNewSettings = (frameworkSettings, newSettings, metadata) => {
/** @type {string[]} */
const message = ['']
const [setting] = newSettings

if (frameworkSettings?.framework !== setting?.framework) {
message.push(
`- Framework does not match:`,
` [old]: ${frameworkSettings?.framework}`,
` [new]: ${setting?.framework}`,
'',
)
}

if (frameworkSettings?.command !== setting?.devCommand) {
message.push(
`- command does not match:`,
` [old]: ${frameworkSettings?.command}`,
` [new]: ${setting?.devCommand}`,
'',
)
}

if (frameworkSettings?.dist !== setting?.dist) {
message.push(`- dist does not match:`, ` [old]: ${frameworkSettings?.dist}`, ` [new]: ${setting?.dist}`, '')
}

if (frameworkSettings?.frameworkPort !== setting?.frameworkPort) {
message.push(
`- frameworkPort does not match:`,
` [old]: ${frameworkSettings?.frameworkPort}`,
` [new]: ${setting?.frameworkPort}`,
'',
)
}

if (message.length !== 0) {
reportError(
{
name: 'NewSettingsDetectionMismatch',
errorMessage: 'New Settings detection does not match old one',
message: message.join('\n'),
},
{ severity: 'info', metadata },
)
}
}

const detectFrameworkSettings = async ({ projectDir }) => {
const projectFrameworks = await listFrameworks({ projectDir })
const frameworks = projectFrameworks.filter((framework) => hasDevCommand(framework))
Expand Down Expand Up @@ -224,7 +293,7 @@ const hasCommandAndTargetPort = ({ devConfig }) => devConfig.command && devConfi
/**
* Creates settings for the custom framework
* @param {*} param0
* @returns {import('./types').BaseServerSettings}
* @returns {import('./types.js').BaseServerSettings}
*/
const handleCustomFramework = ({ devConfig }) => {
if (!hasCommandAndTargetPort({ devConfig })) {
Expand Down Expand Up @@ -271,7 +340,7 @@ const mergeSettings = async ({ devConfig, frameworkSettings = {} }) => {
/**
* Handles a forced framework and retrieves the settings for it
* @param {*} param0
* @returns {Promise<import('./types').BaseServerSettings>}
* @returns {Promise<import('./types.js').BaseServerSettings>}
*/
const handleForcedFramework = async ({ devConfig, projectDir }) => {
// this throws if `devConfig.framework` is not a supported framework
Expand All @@ -281,15 +350,16 @@ const handleForcedFramework = async ({ devConfig, projectDir }) => {

/**
* Get the server settings based on the flags and the devConfig
* @param {import('../commands/dev/types').DevConfig} devConfig
* @param {import('../commands/dev/types.js').DevConfig} devConfig
* @param {import('commander').OptionValues} options
* @param {string} projectDir
* @returns {Promise<import('./types').ServerSettings>}
* @param {Record<string, Record<string, any>>} [metadata]
* @returns {Promise<import('./types.js').ServerSettings>}
*/
const detectServerSettings = async (devConfig, options, projectDir) => {
const detectServerSettings = async (devConfig, options, projectDir, metadata) => {
validateStringProperty({ devConfig, property: 'framework' })

/** @type {Partial<import('./types').BaseServerSettings>} */
/** @type {Partial<import('./types.js').BaseServerSettings>} */
let settings = {}

if (options.dir || devConfig.framework === '#static') {
Expand All @@ -300,6 +370,19 @@ const detectServerSettings = async (devConfig, options, projectDir) => {

const runDetection = !hasCommandAndTargetPort({ devConfig })
const frameworkSettings = runDetection ? await detectFrameworkSettings({ projectDir }) : undefined
const newSettings = runDetection ? await detectSettings(projectDir) : undefined

// just report differences in the settings
detectChangesInNewSettings(frameworkSettings, newSettings || [], {
...metadata,
settings: {
projectDir,
devConfig,
options,
old: frameworkSettings,
settings: newSettings,
},
})

if (frameworkSettings === undefined && runDetection) {
log(`${NETLIFYDEVWARN} No app server detected. Using simple static server`)
Expand Down Expand Up @@ -368,7 +451,7 @@ const formatSettingsArrForInquirer = function (frameworks) {
* Returns a copy of the provided config with any plugins provided by the
* server settings
* @param {*} config
* @param {Partial<import('./types').ServerSettings>} settings
* @param {Partial<import('./types.js').ServerSettings>} settings
* @returns {*} Modified config
*/
export const getConfigWithPlugins = (config, settings) => {
Expand Down
Loading

1 comment on commit add7a6d

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 Benchmark results

  • Package size: 305 MB

Please sign in to comment.