Skip to content

Commit

Permalink
feat: make feature flags available to @netlify/config (#6691)
Browse files Browse the repository at this point in the history
* refactor: construct API client earlier

* refactor: move siteID up

* fix: fetch featureflags in CLI

* chore: add tests for feeding feature flags to config

* chore: fix lint errors

* chore: fix tests

* chore: fix order

* chore: logging

* chore: conditionally get site info for config

* chore: fix get site params

* chore: analyse all flags before getting site

* chore: only call getSite once

* chore: add flags to site obj

* chore: only set flags if site there

* chore: fix undefined issue

* chore: fix link test

* chore: remove logs

* chore: remove only

* Revert changes to Link test

* fix: only request API if there's a token

* fix: fetch fresh siteData only in build and dev

* chore: remove unneeded arg

* fix: only fetch featureflags for build/deploy/dev

* refactor: extract into variable

* chore: rework test

* fix: don't fetch when offline

* fix: only make request if token exists

* fix: prettier

* fix: remove unneeded argument

* fix: share featureflags across invocations by making it an instance variable

* Revert changes to site-by-name fetching

* refactor: remove siteId parameter

* fix: wrap featureflag finding in try/catch

* chore: add readme for help on PR and serve tests

* fix: get site info from name or id, and do it just once

* Revert "chore: add readme for help on PR and serve tests"

This reverts commit 7c25a9f.

* Revert "fix: get site info from name or id, and do it just once"

This reverts commit fbb18b6.

* fix: pass feature flag arg

---------

Co-authored-by: Simon Knott <[email protected]>
Co-authored-by: Lewis Thorley <[email protected]>
Co-authored-by: Netlify Bot <[email protected]>
Co-authored-by: Karin Hendrikse <[email protected]>
  • Loading branch information
5 people committed Jun 14, 2024
1 parent 0f69e06 commit 1706c8c
Show file tree
Hide file tree
Showing 4 changed files with 113 additions and 13 deletions.
42 changes: 31 additions & 11 deletions src/commands/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
sortOptions,
warn,
} from '../utils/command-helpers.js'
import { FeatureFlags } from '../utils/feature-flags.js'
import getGlobalConfig from '../utils/get-global-config.js'
import { getSiteByName } from '../utils/get-site.js'
import openBrowser from '../utils/open-browser.js'
Expand Down Expand Up @@ -70,6 +71,11 @@ const HELP_SEPARATOR_WIDTH = 5
*/
const COMMANDS_WITHOUT_WORKSPACE_OPTIONS = new Set(['api', 'recipes', 'completion', 'status', 'switch', 'login', 'lm'])

/**
* A list of commands where we need to fetch featureflags for config resolution
*/
const COMMANDS_WITH_FEATURE_FLAGS = new Set(['build', 'dev', 'deploy'])

/** Formats a help list correctly with the correct indent */
const formatHelpList = (textArray: string[]) => textArray.join('\n').replace(/^/gm, ' '.repeat(HELP_INDENT_WIDTH))

Expand Down Expand Up @@ -169,6 +175,9 @@ export default class BaseCommand extends Command {
/** The current workspace package we should execute the commands in */
workspacePackage?: string

featureFlags: FeatureFlags = {}
siteId?: string

/**
* IMPORTANT this function will be called for each command!
* Don't do anything expensive in there.
Expand Down Expand Up @@ -551,6 +560,26 @@ export default class BaseCommand extends Command {
process.env.NETLIFY_API_URL === `${apiUrl.protocol}//${apiUrl.host}` ? '/api/v1' : apiUrl.pathname
}

const agent = await getAgent({
httpProxy: flags.httpProxy,
certificateFile: flags.httpProxyCertificateFilename,
})
const apiOpts = { ...apiUrlOpts, agent }
// TODO: remove typecast once we have proper types for the API
const api = new NetlifyAPI(token || '', apiOpts) as NetlifyOptions['api']

actionCommand.siteId = flags.siteId || (typeof flags.site === 'string' && flags.site) || state.get('siteId')

const needsFeatureFlagsToResolveConfig = COMMANDS_WITH_FEATURE_FLAGS.has(actionCommand.name())
if (api.accessToken && !flags.offline && needsFeatureFlagsToResolveConfig && actionCommand.siteId) {
try {
const site = await api.getSite({ siteId: actionCommand.siteId, feature_flags: 'cli' })
actionCommand.featureFlags = site.feature_flags
} catch {
// if the site is not found, that could mean that the user passed a site name, not an ID
}
}

// ==================================================
// Start retrieving the configuration through the
// configuration file and the API
Expand All @@ -561,20 +590,12 @@ export default class BaseCommand extends Command {
packagePath: this.workspacePackage,
// The config flag needs to be resolved from the actual process working directory
configFilePath: packageConfig,
state,
token,
...apiUrlOpts,
})
const { buildDir, config, configPath, env, repositoryRoot, siteInfo } = cachedConfig
env.NETLIFY_CLI_VERSION = { sources: ['internal'], value: version }
const normalizedConfig = normalizeConfig(config)
const agent = await getAgent({
httpProxy: flags.httpProxy,
certificateFile: flags.httpProxyCertificateFilename,
})
const apiOpts = { ...apiUrlOpts, agent }
// TODO: remove typecast once we have proper types for the API
const api = new NetlifyAPI(token || '', apiOpts) as NetlifyOptions['api']

// If a user passes a site name as an option instead of a site ID to options.site, the siteInfo object
// will only have the property siteInfo.id. Checking for one of the other properties ensures that we can do
Expand Down Expand Up @@ -650,8 +671,6 @@ export default class BaseCommand extends Command {
async getConfig(config: {
cwd: string
token?: string | null
// eslint-disable-next-line @typescript-eslint/no-explicit-any
state?: any
offline?: boolean
/** An optional path to the netlify configuration file e.g. netlify.toml */
configFilePath?: string
Expand All @@ -672,14 +691,15 @@ export default class BaseCommand extends Command {
cwd: config.cwd,
context: flags.context || process.env.CONTEXT || this.getDefaultContext(),
debug: flags.debug,
siteId: flags.siteId || (typeof flags.site === 'string' && flags.site) || config.state.get('siteId'),
siteId: this.siteId,
token: config.token,
mode: 'cli',
host: config.host,
pathPrefix: config.pathPrefix,
scheme: config.scheme,
offline: config.offline ?? flags.offline,
siteFeatureFlagPrefix: 'cli',
featureFlags: this.featureFlags,
})
} catch (error_) {
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
Expand Down
1 change: 0 additions & 1 deletion src/commands/dev/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ export const dev = async (options: OptionValues, command: BaseCommand) => {
const { config: newConfig } = await command.getConfig({
cwd: command.workingDir,
offline: true,
state,
})
const normalizedNewConfig = normalizeConfig(newConfig)

Expand Down
2 changes: 1 addition & 1 deletion src/commands/serve/serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export const serve = async (options: OptionValues, command: BaseCommand) => {

// TODO: We should consolidate this with the existing config watcher.
const getUpdatedConfig = async () => {
const { config: newConfig } = await command.getConfig({ cwd: command.workingDir, offline: true, state })
const { config: newConfig } = await command.getConfig({ cwd: command.workingDir, offline: true })
const normalizedNewConfig = normalizeConfig(newConfig)

return normalizedNewConfig
Expand Down
81 changes: 81 additions & 0 deletions tests/integration/commands/build/build-program.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import process from 'process'

import { expect, beforeEach, afterAll, describe, test, vi } from 'vitest'

import BaseCommand from '../../../../src/commands/base-command.ts'
import { createBuildCommand } from '../../../../src/commands/build/index.ts'
import { getEnvironmentVariables, withMockApi } from '../../utils/mock-api.js'
import { withSiteBuilder } from '../../utils/site-builder.ts'

let configOptions = {}

vi.mock('@netlify/config', async (importOriginal) => {
const original = await importOriginal()
return {
...original,
resolveConfig: (options) => {
configOptions = options
return original.resolveConfig(options)
},
}
})

const siteInfo = {
account_slug: 'test-account',
id: 'site_id',
name: 'site-name',
feature_flags: { test_flag: true },
}
const routes = [
{ path: 'sites/site_id', response: siteInfo },
{ path: 'sites/site_id/service-instances', response: [] },
{
path: 'accounts',
response: [{ slug: siteInfo.account_slug }],
},
]
// eslint-disable-next-line workspace/no-process-cwd
const originalCwd = process.cwd
const originalConsoleLog = console.log
const originalEnv = process.env

describe('command/build', () => {
beforeEach(() => {
vi.resetModules()
vi.clearAllMocks()
configOptions = {}
console.log = () => {}
})

afterAll(() => {
// eslint-disable-next-line workspace/no-process-cwd
process.cwd = originalCwd
console.log = originalConsoleLog
process.env = originalEnv

vi.resetModules()
vi.restoreAllMocks()
})

test('should pass feature flags to @netlify/config', async (t) => {
// this ensures that the process.exit does not exit the test process
vi.spyOn(process, 'exit').mockImplementation((code) => {
expect(code).toBe(0)
})
await withSiteBuilder(t, async (builder) => {
// eslint-disable-next-line workspace/no-process-cwd
process.cwd = () => builder.directory
await withMockApi(routes, async ({ apiUrl }) => {
process.env = getEnvironmentVariables({ apiUrl })

await builder.withNetlifyToml({ config: {} }).withStateFile({ siteId: siteInfo.id }).build()

await createBuildCommand(new BaseCommand('netlify')).parseAsync(['', '', 'build'])
expect(configOptions.featureFlags).toEqual(siteInfo.feature_flags)

await createBuildCommand(new BaseCommand('netlify')).parseAsync(['', '', 'build', '--offline'])
expect(configOptions.featureFlags, 'should not call API in offline mode').toEqual({})
})
})
})
})

3 comments on commit 1706c8c

@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,317
  • Package size: 480 MB
  • Number of ts-expect-error directives: 978

@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,317
  • Package size: 480 MB
  • Number of ts-expect-error directives: 978

@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,317
  • Package size: 480 MB
  • Number of ts-expect-error directives: 978

Please sign in to comment.