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: allow frameworks to detect version of CLI that's being used #6226

Merged
merged 14 commits into from
Jan 8, 2024
Merged
38 changes: 20 additions & 18 deletions src/commands/base-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
exit,
getToken,
log,
netlifyCliVersion,
Copy link
Member

Choose a reason for hiding this comment

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

nit: This name feels a bit redundant and overly verbose to me, since everything here pertains to the Netlify CLI and there's no need to make that explicit with a prefix. Could we use something like version?

normalizeConfig,
padLeft,
pollForToken,
Expand All @@ -38,6 +39,9 @@ import openBrowser from '../utils/open-browser.js'
import StateConfig from '../utils/state-config.js'
import { identify, reportError, track } from '../utils/telemetry/index.js'

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type $FIXME = any

// load the autocomplete plugin
inquirer.registerPrompt('autocomplete', inquirerAutocompletePrompt)
/** Netlify CLI client id. Lives in [email protected] */
Expand Down Expand Up @@ -229,7 +233,7 @@ export default class BaseCommand extends Command {
}
debug(`${name}:preAction`)('start')
this.analytics = { startTime: process.hrtime.bigint() }
await this.init(actionCommand)
await this.init(actionCommand as BaseCommand)
debug(`${name}:preAction`)('end')
})
}
Expand Down Expand Up @@ -521,8 +525,7 @@ export default class BaseCommand extends Command {
* @param {BaseCommand} actionCommand The command of the action that is run (`this.` gets the parent command)
* @private
*/
// @ts-expect-error TS(7006) FIXME: Parameter 'actionCommand' implicitly has an 'any' ... Remove this comment to see the full error message
async init(actionCommand) {
async init<C extends BaseCommand>(actionCommand: C) {
debug(`${actionCommand.name()}:init`)('start')
const flags = actionCommand.opts()
// here we actually want to use the process.cwd as we are setting the workingDir
Expand Down Expand Up @@ -606,7 +609,8 @@ export default class BaseCommand extends Command {
token,
...apiUrlOpts,
})
const { buildDir, config, configPath, repositoryRoot, siteInfo } = cachedConfig
const { buildDir, config, configPath, env, repositoryRoot, siteInfo } = cachedConfig
env.NETLIFY_CLI_VERSION = { sources: ['internal'], value: netlifyCliVersion }
Copy link
Contributor Author

@Skn0tt Skn0tt Nov 29, 2023

Choose a reason for hiding this comment

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

This is the main functional change in this PR.

I don't have strong feelings around the name of the var - maybe NETLIFY_CLI would be better? We already inject NETLIFY_LOCAL=true. We could reuse that, but I imagine some consumers are depending on its value being "true".

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this var name is just fine - clear is best imo (even if it's wordy :D )

const normalizedConfig = normalizeConfig(config)
const agent = await getAgent({
httpProxy: flags.httpProxy,
Expand Down Expand Up @@ -689,21 +693,19 @@ export default class BaseCommand extends Command {

/**
* Find and resolve the Netlify configuration
* @param {object} config
* @param {string} config.cwd
* @param {string|null=} config.token
* @param {*} config.state
* @param {boolean=} config.offline
* @param {string=} config.configFilePath An optional path to the netlify configuration file e.g. netlify.toml
* @param {string=} config.packagePath
* @param {string=} config.repositoryRoot
* @param {string=} config.host
* @param {string=} config.pathPrefix
* @param {string=} config.scheme
* @returns {ReturnType<typeof resolveConfig>}
*/
// @ts-expect-error TS(7023) FIXME: 'getConfig' implicitly has return type 'any' becau... Remove this comment to see the full error message
async getConfig(config) {
async getConfig(config: {
cwd: string
token?: string | null
state: $FIXME
offline?: boolean
configFilePath?: string
packagePath?: string
repositoryRoot?: string
host?: string
pathPrefix?: string
scheme?: string
}): Promise<ReturnType<typeof resolveConfig>> {
// the flags that are passed to the command like `--debug` or `--offline`
const flags = this.opts()

Expand Down
6 changes: 4 additions & 2 deletions src/commands/env/env-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,10 @@ export const envList = async (options: OptionValues, command: BaseCommand) => {

// filter out general sources
environment = Object.fromEntries(
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
Object.entries(environment).filter(([, variable]) => variable.sources[0] !== 'general'),
Object.entries(environment).filter(
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
Copy link
Member

Choose a reason for hiding this comment

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

What would it take to get rid of this?

([, variable]) => variable.sources[0] !== 'general' && variable.sources[0] !== 'internal',
),
)

// Return json response for piping commands
Expand Down
3 changes: 2 additions & 1 deletion src/utils/command-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ const arch = os.arch() === 'ia32' ? 'x86' : os.arch()

const { name, version } = await getPackageJson()

export const USER_AGENT = `${name}/${version} ${platform}-${arch} node-${process.version}`
export const netlifyCliVersion = version
export const USER_AGENT = `${name}/${netlifyCliVersion} ${platform}-${arch} node-${process.version}`

/** A list of base command flags that needs to be sorted down on documentation and on help pages */
const BASE_FLAGS = new Set(['--debug', '--httpProxy', '--httpProxyCertificateFilename'])
Expand Down
19 changes: 18 additions & 1 deletion tests/integration/commands/build/build.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ const runBuildCommand = async function (
outputs = [outputs]
}
outputs.forEach((output) => {
t.expect(all.includes(output), `Output of build command does not include '${output}'`).toBe(true)
if (output instanceof RegExp) {
t.expect(all).toMatch(output)
} else {
t.expect(all.includes(output), `Output of build command does not include '${output}'`).toBe(true)
}
})
t.expect(exitCode).toBe(expectedExitCode)
}
Expand Down Expand Up @@ -286,4 +290,17 @@ describe.concurrent('command/build', () => {
})
})
})

test('should have version in NETLIFY_CLI_VERSION variable', async (t) => {
await withSiteBuilder('NETLIFY_CLI_VERSION-env', async (builder) => {
await builder
.withNetlifyToml({ config: { build: { command: 'echo NETLIFY_CLI_VERSION=$NETLIFY_CLI_VERSION' } } })
.build()

await runBuildCommand(t, builder.directory, {
output: /NETLIFY_CLI_VERSION=\d+\.\d+.\d+/,
flags: ['--offline'],
})
})
})
})
11 changes: 11 additions & 0 deletions tests/integration/commands/dev/dev.config.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,17 @@ describe.concurrent('commands/dev/config', () => {
})
})

test('should provide CLI version in env var', async (t) => {
await withSiteBuilder('site-with-netlify-version-env-var', async (builder) => {
await builder.withNetlifyToml({ config: { dev: { command: `node -e console.log(process.env)` } } }).build()

await withDevServer({ cwd: builder.directory }, async (server) => {
await server.close()
t.expect(server.output).toContain('NETLIFY_CLI_VERSION')
})
})
})

test('should set value of the CONTEXT env variable', async (t) => {
await withSiteBuilder('site-with-context-override', async (builder) => {
builder.withNetlifyToml({ config: { functions: { directory: 'functions' } } }).withFunction({
Expand Down
Loading