Skip to content

Commit

Permalink
fix: use heuristics for build and deploy command as well (#6729)
Browse files Browse the repository at this point in the history
* fix: use heuristics for build and deploy command as well

* chore: fix case where no plugins recommended are in the settings

* chore: fix comment

* chore: add integration tests for build and deploy

* chore: prepare test fixture on ci

* chore: update min node version for next.js
  • Loading branch information
lukasholzer committed Jul 15, 2024
1 parent e45d378 commit 111967c
Show file tree
Hide file tree
Showing 24 changed files with 1,149 additions and 52 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ jobs:
matrix:
os: [ubuntu-latest, macOS-latest, windows-latest]
# Pinning 20.x version as a temporary workaround due to this https://github.com/nodejs/node/issues/52884
node-version: ['18.14.0', '20.12.2', '22']
node-version: ['18.17.0', '20.12.2', '22']
shard: ['1/4', '2/4', '3/4', '4/4']

exclude:
- os: macOS-latest
node-version: '18.14.0'
node-version: '18.17.0'
- os: windows-latest
node-version: '18.14.0'
node-version: '18.17.0'
- os: windows-latest
node-version: '22'
fail-fast: false
Expand Down
112 changes: 112 additions & 0 deletions package-lock.json

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

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"test:init:cli-help": "npm run start -- --help",
"test:init:eleventy-deps": "cd tests/integration/__fixtures__/eleventy-site && pnpm install --frozen-lockfile",
"test:init:hugo-deps": "npm ci --prefix tests/integration/__fixtures__/hugo-site --no-audit",
"test:init:next-deps": "npm ci --prefix tests/integration/__fixtures__/next-app-without-config --no-audit",
"test:dev:vitest": "vitest run tests/unit/ && vitest run tests/integration",
"test:ci:vitest:unit": "vitest run --coverage tests/unit/",
"test:ci:vitest:integration": "vitest run --coverage tests/integration/",
Expand Down Expand Up @@ -201,6 +202,7 @@
"@types/ws": "8.5.10",
"@vitest/coverage-v8": "1.6.0",
"c8": "9.1.0",
"cheerio": "^1.0.0-rc.12",
"eslint-plugin-sort-destructure-keys": "2.0.0",
"eslint-plugin-workspace": "file:./tools/lint-rules",
"fast-glob": "3.3.2",
Expand Down
9 changes: 2 additions & 7 deletions src/commands/build/build.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { OptionValues } from 'commander'

import { getBuildOptions, runBuild } from '../../lib/build.js'
import { detectFrameworkSettings } from '../../utils/build-info.js'
import { detectFrameworkSettings, getDefaultConfig } from '../../utils/build-info.js'
import { error, exit, getToken } from '../../utils/command-helpers.js'
import { getEnvelopeEnv } from '../../utils/env/index.js'
import BaseCommand from '../base-command.js'
Expand Down Expand Up @@ -31,14 +31,9 @@ export const build = async (options: OptionValues, command: BaseCommand) => {
const [token] = await getToken()
const settings = await detectFrameworkSettings(command, 'build')

// override the build command with the detection result if no command is specified through the config
if (!cachedConfig.config.build.command) {
cachedConfig.config.build.command = settings?.buildCommand
cachedConfig.config.build.commandOrigin = 'heuristics'
}

const buildOptions = await getBuildOptions({
cachedConfig,
defaultConfig: getDefaultConfig(settings),
packagePath: command.workspacePackage,
currentDir: command.workingDir,
token,
Expand Down
6 changes: 5 additions & 1 deletion src/commands/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { featureFlags as edgeFunctionsFeatureFlags } from '../../lib/edge-functi
import { normalizeFunctionsConfig } from '../../lib/functions/config.js'
import { BACKGROUND_FUNCTIONS_WARNING } from '../../lib/log.js'
import { startSpinner, stopSpinner } from '../../lib/spinner.js'
import { detectFrameworkSettings, getDefaultConfig } from '../../utils/build-info.js'
import {
NETLIFYDEV,
NETLIFYDEVERR,
Expand Down Expand Up @@ -558,14 +559,15 @@ const runDeploy = async ({
* @returns
*/
// @ts-expect-error TS(7031) FIXME: Binding element 'cachedConfig' implicitly has an '... Remove this comment to see the full error message
const handleBuild = async ({ cachedConfig, currentDir, deployHandler, options, packagePath }) => {
const handleBuild = async ({ cachedConfig, currentDir, defaultConfig, deployHandler, options, packagePath }) => {
if (!options.build) {
return {}
}
// @ts-expect-error TS(2554) FIXME: Expected 1 arguments, but got 0.
const [token] = await getToken()
const resolvedOptions = await getBuildOptions({
cachedConfig,
defaultConfig,
packagePath,
token,
options,
Expand Down Expand Up @@ -784,6 +786,7 @@ export const deploy = async (options: OptionValues, command: BaseCommand) => {
const { workingDir } = command
const { api, site, siteInfo } = command.netlify
const alias = options.alias || options.branch
const settings = await detectFrameworkSettings(command, 'build')

command.setAnalyticsPayload({ open: options.open, prod: options.prod, json: options.json, alias: Boolean(alias) })

Expand Down Expand Up @@ -847,6 +850,7 @@ export const deploy = async (options: OptionValues, command: BaseCommand) => {
await handleBuild({
packagePath: command.workspacePackage,
cachedConfig: command.netlify.cachedConfig,
defaultConfig: getDefaultConfig(settings),
currentDir: command.workingDir,
options,
// @ts-expect-error TS(7031) FIXME: Binding element 'netlifyConfig' implicitly has an ... Remove this comment to see the full error message
Expand Down
3 changes: 3 additions & 0 deletions src/lib/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ export const getBuildOptions = async ({
cachedConfig,
// @ts-expect-error TS(7031) FIXME: Binding element 'currentDir' implicitly has an 'an... Remove this comment to see the full error message
currentDir,
// @ts-expect-error TS(7031) FIXME: Binding element 'defaultConfig' implicitly has an '... Remove this comment to see the full error message
defaultConfig,
// @ts-expect-error TS(7031) FIXME: Binding element 'deployHandler' implicitly has an ... Remove this comment to see the full error message
deployHandler,
// @ts-expect-error TS(7031) FIXME: Binding element 'context' implicitly has an 'any' ... Remove this comment to see the full error message
Expand Down Expand Up @@ -71,6 +73,7 @@ export const getBuildOptions = async ({

return {
cachedConfig,
defaultConfig,
siteId: cachedConfig.siteInfo.id,
packagePath,
token,
Expand Down
29 changes: 29 additions & 0 deletions src/utils/build-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import fuzzy from 'fuzzy'
import inquirer from 'inquirer'

import BaseCommand from '../commands/base-command.js'
import { $TSFixMe } from '../commands/types.js'

import { chalk, log } from './command-helpers.js'

Expand Down Expand Up @@ -122,3 +123,31 @@ command = "${chosenSettings.devCommand}"
return chosenSettings
}
}

/**
* Generates a defaultConfig for @netlify/build based on the settings from the heuristics
* Returns the defaultConfig in the format that @netlify/build expects (json version of toml)
* @param settings The settings from the heuristics
*/
export const getDefaultConfig = (settings?: Settings): $TSFixMe | undefined => {
if (!settings) {
return undefined
}

// TODO: We need proper types for the netlify configuration
const config: $TSFixMe = { build: {} }

if (settings.buildCommand) {
config.build.command = settings.buildCommand
config.build.commandOrigin = 'default'
}

if (settings.dist) {
config.build.publish = settings.dist
config.build.publishOrigin = 'default'
}

config.plugins = settings.plugins_recommended?.map((plugin) => ({ package: plugin })) || []

return config
}
4 changes: 3 additions & 1 deletion src/utils/deploy/deploy-site.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,9 @@ export const deploySite = async (
}

if (functionsWithNativeModules.length !== 0) {
const functionsWithNativeModulesMessage = functionsWithNativeModules.map(({ name }) => `- ${name}`).join('\n')
const functionsWithNativeModulesMessage = functionsWithNativeModules
.map(({ name }: { name: string }) => `- ${name}`)
.join('\n')
warn(`Modules with native dependencies\n
${functionsWithNativeModulesMessage}
Expand Down
4 changes: 2 additions & 2 deletions src/utils/deploy/hash-fns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const hashFns = async (
statusCb: $TSFixMe
tmpDir: $TSFixMe
},
) => {
): Promise<$TSFixMe> => {
const {
assetType = 'function',
concurrentHash,
Expand Down Expand Up @@ -166,8 +166,8 @@ const hashFns = async (
priority,
runtime,
runtimeVersion,
trafficRules,
timeout,
trafficRules,
}) => ({
filepath: functionPath,
root: tmpDir,
Expand Down
3 changes: 2 additions & 1 deletion src/utils/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ export const isFeatureFlagEnabled = (flagName: string, siteInfo): boolean =>
export const getFeatureFlagsFromSiteInfo = (siteInfo: {
feature_flags?: Record<string, boolean | string | number>
}): FeatureFlags => ({
...(siteInfo.feature_flags || {}),
...siteInfo.feature_flags,
// see https://github.com/netlify/pod-dev-foundations/issues/581#issuecomment-1731022753
zisi_golang_use_al2: isFeatureFlagEnabled('cli_golang_use_al2', siteInfo),
netlify_build_frameworks_api: true,
project_ceruledge_ui: true,
})

export type FeatureFlags = Record<string, boolean | string | number>
36 changes: 36 additions & 0 deletions tests/integration/__fixtures__/next-app-without-config/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.

# dependencies
/node_modules
/.pnp
.pnp.js
.yarn/install-state.gz

# testing
/coverage

# next.js
/.next/
/out/

# production
/build

# misc
.DS_Store
*.pem

# debug
npm-debug.log*
yarn-debug.log*
yarn-error.log*

# local env files
.env*.local

# vercel
.vercel

# typescript
*.tsbuildinfo
next-env.d.ts
Binary file not shown.
Loading

2 comments on commit 111967c

@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

  • Dependency count: 1,212
  • Package size: 313 MB
  • Number of ts-expect-error directives: 977

@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

  • Dependency count: 1,212
  • Package size: 313 MB
  • Number of ts-expect-error directives: 977

Please sign in to comment.