Skip to content

feat(build-info): pass feature flags as json and change plugins to an object #5449

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/build-info/src/build-systems/nx.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ describe('nx-integrated project.json based', () => {
frameworkPort: 4200,
name: `Nx + Next.js ${join('packages/website')}`,
packagePath: join('packages/website'),
plugins_recommended: ['@netlify/plugin-nextjs'],
plugins: [{ name: '@netlify/plugin-nextjs', alwaysInstall: true }],
}),
]),
)
Expand All @@ -212,7 +212,7 @@ describe('nx-integrated project.json based', () => {
frameworkPort: 3000,
name: `Nx + Astro ${join('packages/astro')}`,
packagePath: join('packages/astro'),
plugins_recommended: [],
plugins: [],
}),
]),
)
Expand Down Expand Up @@ -250,7 +250,7 @@ describe('nx-integrated workspace.json based', () => {
frameworkPort: 4200,
name: `Nx + React Static ${join('apps/website')}`,
packagePath: join('apps/website'),
plugins_recommended: [],
plugins: [],
}),
]),
)
Expand All @@ -264,7 +264,7 @@ describe('nx-integrated workspace.json based', () => {
framework: { id: 'astro', name: 'Astro' },
name: `Nx + Astro ${join('apps/astro')}`,
packagePath: join('apps/astro'),
plugins_recommended: [],
plugins: [],
}),
]),
)
Expand Down
2 changes: 1 addition & 1 deletion packages/build-info/src/frameworks/angular.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test('should detect Angular', async ({ fs }) => {
expect(detected?.[0].build.command).toBe('ng build --prod')
expect(detected?.[0].build.directory).toBe(fs.join('dist', 'demo', 'browser'))
expect(detected?.[0].dev?.command).toBe('ng serve')
expect(detected?.[0].plugins).toEqual(['@netlify/angular-runtime'])
expect(detected?.[0].plugins).toEqual([{ name: '@netlify/angular-runtime' }])
})

test('should only install plugin on v17+', async ({ fs }) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/build-info/src/frameworks/angular.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class Angular extends BaseFramework implements Framework {

if (this.detected) {
if (this.version && gte(this.version, '17.0.0-rc')) {
this.plugins.push('@netlify/angular-runtime')
this.plugins.push({ name: '@netlify/angular-runtime' })
const angularJson = await this.project.fs.gracefullyReadFile('angular.json')
if (angularJson) {
const { projects, defaultProject } = JSON.parse(angularJson)
Expand Down
13 changes: 10 additions & 3 deletions packages/build-info/src/frameworks/framework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ export type Detection = {

export type FrameworkInfo = ReturnType<Framework['toJSON']>

export type BuildPlugin = {
name: string
/** Plugins that should be always installed */
alwaysInstall?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a bit more context to this property? When will it be used? I don't see it being pulled in yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reference is in the PR description

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, we have a lot of hardcoded instances of the next runtime inside the react UI and the CLI, where we determine if it needs to be auto-installed:

https://github.com/netlify/netlify-react-ui/blob/main/apps/netlify-react-ui/src/actions/repos.ts#L219-L239
https://github.com/netlify/cli/blob/main/src/utils/init/utils.ts#L19-L43

source?: 'toml'
}

export interface Framework {
project: Project

Expand Down Expand Up @@ -67,7 +74,7 @@ export interface Framework {
light?: string
dark?: string
}
plugins: string[]
plugins: BuildPlugin[]
env: Record<string, string>

detect(): Promise<DetectedFramework | undefined>
Expand Down Expand Up @@ -147,7 +154,7 @@ export abstract class BaseFramework implements Framework {
configFiles: string[] = []
npmDependencies: string[] = []
excludedNpmDependencies: string[] = []
plugins: string[] = []
plugins: BuildPlugin[] = []
staticAssetsDirectory?: string
env = {}
dev?: {
Expand Down Expand Up @@ -355,7 +362,7 @@ export abstract class BaseFramework implements Framework {
{},
)
: undefined,
plugins: this.plugins,
plugins: this.plugins.map(({ name }) => name),
}
}
}
7 changes: 3 additions & 4 deletions packages/build-info/src/frameworks/gatsby.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ test('should not add the plugin if the node version is below 12.13.0', async ({
fs.cwd = cwd
const detected = await new Project(fs, cwd).setNodeVersion('12.12.9').detectFrameworks()
expect(detected?.[0].id).toBe('gatsby')
expect(detected?.[0].plugins).toMatchObject([])
expect(detected?.[0].plugins).toHaveLength(0)
})

test('should detect a simple Gatsby project and add the plugin if the node version is large enough', async ({ fs }) => {
Expand All @@ -27,7 +27,7 @@ test('should detect a simple Gatsby project and add the plugin if the node versi
fs.cwd = cwd
const detected = await new Project(fs, cwd).setNodeVersion('12.13.0').detectFrameworks()
expect(detected?.[0].id).toBe('gatsby')
expect(detected?.[0].plugins).toMatchObject(['@netlify/plugin-gatsby'])
expect(detected?.[0].plugins).toMatchObject([{ name: '@netlify/plugin-gatsby' }])
})

test('should detect a simple Gatsby 4 project', async ({ fs }) => {
Expand Down Expand Up @@ -67,8 +67,7 @@ test('should detect a simple Gatsby 4 project', async ({ fs }) => {
frameworkPort: 8000,
name: 'Gatsby',
packagePath: '',
plugins_from_config_file: [],
plugins_recommended: [],
plugins: [],
pollingStrategies: ['TCP', 'HTTP'],
},
])
Expand Down
2 changes: 1 addition & 1 deletion packages/build-info/src/frameworks/gatsby.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export class Gatsby extends BaseFramework implements Framework {

const nodeVersion = await this.project.getCurrentNodeVersion()
if (nodeVersion && gte(nodeVersion, '12.13.0')) {
this.plugins.push('@netlify/plugin-gatsby')
this.plugins.push({ name: '@netlify/plugin-gatsby' })
}
return this as DetectedFramework
}
Expand Down
54 changes: 50 additions & 4 deletions packages/build-info/src/frameworks/next.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,15 @@ describe('Next.js Plugin', () => {
const project = new Project(fs, cwd).setNodeVersion('v10.13.0')
const frameworks = await project.detectFrameworks()
expect(frameworks?.[0].id).toBe('next')
expect(frameworks?.[0].plugins).toEqual(['@netlify/plugin-nextjs'])
expect(frameworks?.[0].plugins).toEqual([{ name: '@netlify/plugin-nextjs', alwaysInstall: true }])
})

test('Should use the old runtime if the next.js version is not >= 13.5.0', async ({ fs, cwd }) => {
const project = new Project(fs, cwd).setNodeVersion('v18.0.0')
project.featureFlags = { project_ceruledge_ui: '@netlify/next-runtime' }
const frameworks = await project.detectFrameworks()
expect(frameworks?.[0].id).toBe('next')
expect(frameworks?.[0].plugins).toEqual([{ name: '@netlify/plugin-nextjs', alwaysInstall: true }])
})

test('Should not detect Next.js plugin for Next.js if when Node version < 10.13.0', async ({ fs, cwd }) => {
Expand All @@ -49,6 +57,44 @@ describe('Next.js Plugin', () => {
})
})

describe('New Next.js Runtime', () => {
beforeEach((ctx) => {
ctx.cwd = mockFileSystem({
'package.json': JSON.stringify({
name: 'my-next-app',
version: '0.1.0',
private: true,
scripts: {
dev: 'next dev',
build: 'next build',
start: 'next start',
},
dependencies: {
next: '13.5.0',
react: '17.0.1',
'react-dom': '17.0.1',
},
}),
})
})

test('Should not use the new runtime if the node version is below 18', async ({ fs, cwd }) => {
const project = new Project(fs, cwd).setNodeVersion('v16.0.0')
project.featureFlags = { project_ceruledge_ui: '@netlify/next-runtime@latest' }
const frameworks = await project.detectFrameworks()
expect(frameworks?.[0].id).toBe('next')
expect(frameworks?.[0].plugins).toEqual([{ name: '@netlify/plugin-nextjs', alwaysInstall: true }])
})

test('Should use the old runtime if the next.js version is not >= 13.5.0', async ({ fs, cwd }) => {
const project = new Project(fs, cwd).setNodeVersion('v18.0.0')
project.featureFlags = { project_ceruledge_ui: '@netlify/next-runtime@latest' }
const frameworks = await project.detectFrameworks()
expect(frameworks?.[0].id).toBe('next')
expect(frameworks?.[0].plugins).toEqual([{ name: '@netlify/next-runtime@latest', alwaysInstall: true }])
})
})

describe('simple Next.js project', async () => {
beforeEach((ctx) => {
ctx.cwd = mockFileSystem({
Expand Down Expand Up @@ -90,7 +136,7 @@ describe('simple Next.js project', async () => {
test('Should detect Next.js plugin for Next.js if when Node version >= 10.13.0', async ({ fs, cwd }) => {
const detected = await new Project(fs, cwd).setEnvironment({ NODE_VERSION: '18.x' }).detectFrameworks()
expect(detected?.[0].id).toBe('next')
expect(detected?.[0].plugins).toMatchObject(['@netlify/plugin-nextjs'])
expect(detected?.[0].plugins).toMatchObject([{ name: '@netlify/plugin-nextjs', alwaysInstall: true }])
})
})

Expand Down Expand Up @@ -134,7 +180,7 @@ describe('Nx monorepo', () => {
devCommand: 'nx run website:serve',
dist: join('dist/packages/website'),
frameworkPort: 4200,
plugins_recommended: ['@netlify/plugin-nextjs'],
plugins: [{ name: '@netlify/plugin-nextjs', alwaysInstall: true }],
})
})
})
Expand All @@ -152,7 +198,7 @@ describe('Nx turborepo', () => {
devCommand: 'turbo run dev --filter web',
dist: join('apps/web/.next'),
frameworkPort: 3000,
plugins_recommended: ['@netlify/plugin-nextjs'],
plugins: [{ name: '@netlify/plugin-nextjs', alwaysInstall: true }],
})
})
})
13 changes: 11 additions & 2 deletions packages/build-info/src/frameworks/next.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,17 @@ export class Next extends BaseFramework implements Framework {

if (this.detected) {
const nodeVersion = await this.project.getCurrentNodeVersion()
if (nodeVersion && gte(nodeVersion, '10.13.0')) {
this.plugins.push('@netlify/plugin-nextjs')
const runtimeFromRollout = this.project.featureFlags['project_ceruledge_ui']
if (
nodeVersion &&
gte(nodeVersion, '18.0.0') &&
this.detected.package?.version &&
gte(this.detected.package.version, '13.5.0') &&
typeof runtimeFromRollout === 'string'
) {
this.plugins.push({ name: runtimeFromRollout ?? '@netlify/plugin-nextjs', alwaysInstall: true })
} else if (nodeVersion && gte(nodeVersion, '10.13.0')) {
this.plugins.push({ name: '@netlify/plugin-nextjs', alwaysInstall: true })
}
return this as DetectedFramework
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ exports[`should retrieve the build info for providing a rootDir 1`] = `
"frameworkPort": 3000,
"name": "PNPM + Astro packages/blog",
"packagePath": "packages/blog",
"plugins_from_config_file": [],
"plugins_recommended": [],
"plugins": [],
"pollingStrategies": [
"TCP",
"HTTP",
Expand All @@ -84,9 +83,11 @@ exports[`should retrieve the build info for providing a rootDir 1`] = `
"frameworkPort": 3000,
"name": "PNPM + Next.js packages/website",
"packagePath": "packages/website",
"plugins_from_config_file": [],
"plugins_recommended": [
"@netlify/plugin-nextjs",
"plugins": [
{
"alwaysInstall": true,
"name": "@netlify/plugin-nextjs",
},
],
"pollingStrategies": [
"TCP",
Expand Down Expand Up @@ -199,8 +200,7 @@ exports[`should retrieve the build info for providing a rootDir and a nested pro
"frameworkPort": 3000,
"name": "PNPM + Astro packages/blog",
"packagePath": "packages/blog",
"plugins_from_config_file": [],
"plugins_recommended": [],
"plugins": [],
"pollingStrategies": [
"TCP",
"HTTP",
Expand Down Expand Up @@ -274,8 +274,7 @@ exports[`should retrieve the build info for providing a rootDir and the same pro
"frameworkPort": 3000,
"name": "PNPM + Astro packages/blog",
"packagePath": "packages/blog",
"plugins_from_config_file": [],
"plugins_recommended": [],
"plugins": [],
"pollingStrategies": [
"TCP",
"HTTP",
Expand All @@ -294,9 +293,11 @@ exports[`should retrieve the build info for providing a rootDir and the same pro
"frameworkPort": 3000,
"name": "PNPM + Next.js packages/website",
"packagePath": "packages/website",
"plugins_from_config_file": [],
"plugins_recommended": [
"@netlify/plugin-nextjs",
"plugins": [
{
"alwaysInstall": true,
"name": "@netlify/plugin-nextjs",
},
],
"pollingStrategies": [
"TCP",
Expand Down
23 changes: 14 additions & 9 deletions packages/build-info/src/node/bin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import { report } from '../metrics.js'
import { getBuildInfo } from './get-build-info.js'
import { initializeMetrics } from './metrics.js'

type Args = Arguments<{ projectDir?: string; rootDir?: string; featureFlags: Record<string, boolean> }>
type Args = Arguments<{
projectDir?: string
rootDir?: string
featureFlags: Record<string, number | boolean | string>
}>

yargs(hideBin(argv))
.command(
Expand All @@ -22,13 +26,9 @@ yargs(hideBin(argv))
},
featureFlags: {
string: true,
describe: 'comma separated list of feature flags',
coerce: (value = '') =>
value
.split(',')
.map((flag) => flag.trim())
.filter((flag) => flag.length)
.reduce((prev, cur) => ({ ...prev, [cur]: true }), {}),
describe: 'JSON stringified list of feature flags with values',
alias: 'ff',
coerce: (value: '{}') => JSON.parse(value),
},
}),
async ({ projectDir, rootDir, featureFlags }: Args) => {
Expand All @@ -37,7 +37,12 @@ yargs(hideBin(argv))
try {
console.log(
JSON.stringify(
await getBuildInfo({ projectDir, rootDir, featureFlags, bugsnagClient }),
await getBuildInfo({
projectDir,
rootDir,
featureFlags,
bugsnagClient,
}),
// hide null values from the string output as we use null to identify it has already run but did not detect anything
// undefined marks that it was never run
(_, value) => (value !== null ? value : undefined),
Expand Down
3 changes: 2 additions & 1 deletion packages/build-info/src/node/get-build-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export async function getBuildInfo(
config: {
projectDir?: string
rootDir?: string
featureFlags?: Record<string, boolean>
featureFlags?: Record<string, boolean | number | string>
bugsnagClient?: Client
} = { featureFlags: {} },
): Promise<Info> {
Expand All @@ -54,6 +54,7 @@ export async function getBuildInfo(
fs.logger = new NoopLogger()
const project = new Project(fs, config.projectDir, config.rootDir)
.setBugsnag(config.bugsnagClient)
.setFeatureFlags(config.featureFlags)
.setEnvironment(process.env)
.setNodeVersion(process.version)

Expand Down
7 changes: 7 additions & 0 deletions packages/build-info/src/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ export class Project {
bugsnag: Client
/** A logging instance */
logger: Logger
/** A list of enabled feature flags */
featureFlags: Record<string, boolean | number | string> = {}

/** A function that is used to report errors */
reportFn: typeof report = report
Expand All @@ -76,6 +78,11 @@ export class Project {
return this
}

setFeatureFlags(flags: Record<string, boolean | number | string> = {}): this {
this.featureFlags = { ...this.featureFlags, ...flags }
return this
}

async getCurrentNodeVersion(): Promise<SemVer | null> {
if (this._nodeVersion) {
return this._nodeVersion
Expand Down
Loading