From af0e1bcb7112448d7c047d278d891dea4d7ee9a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eduardo=20Bou=C3=A7as?= Date: Tue, 10 Aug 2021 13:36:45 +0100 Subject: [PATCH] feat: install function template deps in site-level `package.json` (#3125) * feat: install deps of function templates in site-level manifest * refactor: use absolute path in functionPackageJson * chore: add tests * refactor: use array spread * refactor: use additional npm install flags * chore: minor updates * refactor: make sort order stable --- src/commands/functions/create.js | 81 ++++++++++++++--- tests/command.functions.test.js | 148 ++++++++++++++++++++++++++++++- 2 files changed, 214 insertions(+), 15 deletions(-) diff --git a/src/commands/functions/create.js b/src/commands/functions/create.js index e566fd68fa4..7284f10703a 100644 --- a/src/commands/functions/create.js +++ b/src/commands/functions/create.js @@ -7,6 +7,8 @@ const { promisify } = require('util') const { flags: flagsLib } = require('@oclif/command') const chalk = require('chalk') const copy = promisify(require('copy-template-dir')) +const execa = require('execa') +const findUp = require('find-up') const fuzzy = require('fuzzy') const inquirer = require('inquirer') const inquirerAutocompletePrompt = require('inquirer-autocomplete-prompt') @@ -116,10 +118,19 @@ const formatRegistryArrayForInquirer = function (lang) { .filter((folderName) => !folderName.endsWith('.md')) // eslint-disable-next-line node/global-require, import/no-dynamic-require .map((folderName) => require(path.join(templatesDir, lang, folderName, '.netlify-function-template.js'))) - .sort( - (folderNameA, folderNameB) => - (folderNameA.priority || DEFAULT_PRIORITY) - (folderNameB.priority || DEFAULT_PRIORITY), - ) + .sort((folderNameA, folderNameB) => { + const priorityDiff = (folderNameA.priority || DEFAULT_PRIORITY) - (folderNameB.priority || DEFAULT_PRIORITY) + + if (priorityDiff !== 0) { + return priorityDiff + } + + // This branch is needed because `Array.prototype.sort` was not stable + // until Node 11, so the original sorting order from `fs.readdirSync` + // was not respected. We can simplify this once we drop support for + // Node 10. + return folderNameA - folderNameB + }) .map((t) => { t.lang = lang return { @@ -301,12 +312,52 @@ const downloadFromURL = async function (context, flags, args, functionsDir) { } } -const installDeps = function (functionPath) { - return new Promise((resolve) => { - cp.exec('npm i', { cwd: path.join(functionPath) }, () => { - resolve() - }) - }) +// Takes a list of existing packages and a list of packages required by a +// function, and returns the packages from the latter that aren't present +// in the former. The packages are returned as an array of strings with the +// name and version range (e.g. '@netlify/functions@0.1.0'). +const getNpmInstallPackages = (existingPackages = {}, neededPackages = {}) => + Object.entries(neededPackages) + .filter(([name]) => existingPackages[name] === undefined) + .map(([name, version]) => `${name}@${version}`) + +// When installing a function's dependencies, we first try to find a site-level +// `package.json` file. If we do, we look for any dependencies of the function +// that aren't already listed as dependencies of the site and install them. If +// we don't do this check, we may be upgrading the version of a module used in +// another part of the project, which we don't want to do. +const installDeps = async ({ functionPackageJson, functionPath, functionsDir }) => { + // eslint-disable-next-line import/no-dynamic-require, node/global-require + const { dependencies: functionDependencies, devDependencies: functionDevDependencies } = require(functionPackageJson) + const sitePackageJson = await findUp('package.json', { cwd: functionsDir }) + const npmInstallFlags = ['--no-audit', '--no-fund'] + + // If there is no site-level `package.json`, we fall back to the old behavior + // of keeping that file in the function directory and running `npm install` + // from there. + if (!sitePackageJson) { + await execa('npm', ['i', ...npmInstallFlags], { cwd: functionPath }) + + return + } + + // eslint-disable-next-line import/no-dynamic-require, node/global-require + const { dependencies: siteDependencies, devDependencies: siteDevDependencies } = require(sitePackageJson) + const dependencies = getNpmInstallPackages(siteDependencies, functionDependencies) + const devDependencies = getNpmInstallPackages(siteDevDependencies, functionDevDependencies) + const npmInstallPath = path.dirname(sitePackageJson) + + if (dependencies.length !== 0) { + await execa('npm', ['i', ...dependencies, '--save', ...npmInstallFlags], { cwd: npmInstallPath }) + } + + if (devDependencies.length !== 0) { + await execa('npm', ['i', ...devDependencies, '--save-dev', ...npmInstallFlags], { cwd: npmInstallPath }) + } + + // We installed the function's dependencies in the site-level `package.json`, + // so there's no reason to keep the one copied over from the template. + fs.unlinkSync(functionPackageJson) } // no --url flag specified, pick from a provided template @@ -353,14 +404,16 @@ const scaffoldFromTemplate = async function (context, flags, args, functionsDir) // log('from ', pathToTemplate, ' to ', functionPath) // SWYX: TODO const vars = { NETLIFY_STUFF_TO_REPLACE: 'REPLACEMENT' } - let hasPackageJSON = false + let functionPackageJson const createdFiles = await copy(pathToTemplate, functionPath, vars) createdFiles.forEach((filePath) => { if (filePath.endsWith('.netlify-function-template.js')) return context.log(`${NETLIFYDEVLOG} ${chalk.greenBright('Created')} ${filePath}`) fs.chmodSync(path.resolve(filePath), TEMPLATE_PERMISSIONS) - if (filePath.includes('package.json')) hasPackageJSON = true + if (filePath.includes('package.json')) { + functionPackageJson = path.resolve(filePath) + } }) // delete function template file that was copied over by copydir fs.unlinkSync(path.join(functionPath, '.netlify-function-template.js')) @@ -369,12 +422,12 @@ const scaffoldFromTemplate = async function (context, flags, args, functionsDir) fs.renameSync(path.join(functionPath, `${templateName}.js`), path.join(functionPath, `${name}.js`)) } // npm install - if (hasPackageJSON) { + if (functionPackageJson !== undefined) { const spinner = ora({ text: `installing dependencies for ${name}`, spinner: 'moon', }).start() - await installDeps(functionPath) + await installDeps({ functionPackageJson, functionPath, functionsDir }) spinner.succeed(`installed dependencies for ${name}`) } diff --git a/tests/command.functions.test.js b/tests/command.functions.test.js index de14c954ef5..4b295f272e4 100644 --- a/tests/command.functions.test.js +++ b/tests/command.functions.test.js @@ -11,7 +11,7 @@ const callCli = require('./utils/call-cli') const cliPath = require('./utils/cli-path') const { withDevServer } = require('./utils/dev-server') const got = require('./utils/got') -const { handleQuestions, answerWithValue, CONFIRM } = require('./utils/handle-questions') +const { handleQuestions, answerWithValue, CONFIRM, DOWN } = require('./utils/handle-questions') const { withMockApi } = require('./utils/mock-api') const { killProcess } = require('./utils/process') const { withSiteBuilder } = require('./utils/site-builder') @@ -97,6 +97,152 @@ test('should create a new function directory when none is found', async (t) => { }) }) +test('should install function template dependencies on a site-level `package.json` if one is found', async (t) => { + const siteInfo = { + admin_url: 'https://app.netlify.com/sites/site-name/overview', + ssl_url: 'https://site-name.netlify.app/', + id: 'site_id', + name: 'site-name', + build_settings: { repo_url: 'https://github.com/owner/repo' }, + } + + const routes = [ + { + path: 'accounts', + response: [{ slug: 'test-account' }], + }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'sites/site_id', response: siteInfo }, + { + path: 'sites', + response: [siteInfo], + }, + { path: 'sites/site_id', method: 'patch', response: {} }, + ] + + await withSiteBuilder('site-with-no-functions-dir-with-package-json', async (builder) => { + builder.withPackageJson({ + packageJson: { + dependencies: { + '@netlify/functions': '^0.1.0', + }, + }, + }) + + await builder.buildAsync() + + const createFunctionQuestions = [ + { + question: 'Enter the path, relative to your site', + answer: answerWithValue('test/functions'), + }, + { + question: 'Pick a template', + answer: answerWithValue(`${DOWN}${CONFIRM}`), + }, + { + question: 'name your function', + answer: answerWithValue(CONFIRM), + }, + ] + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['functions:create'], { + env: { + NETLIFY_API_URL: apiUrl, + NETLIFY_SITE_ID: 'site_id', + NETLIFY_AUTH_TOKEN: 'fake-token', + }, + cwd: builder.directory, + }) + + handleQuestions(childProcess, createFunctionQuestions) + + await childProcess + + // eslint-disable-next-line import/no-dynamic-require, node/global-require + const { dependencies } = require(`${builder.directory}/package.json`) + + // NOTE: Ideally we should be running this test with a specific template, + // but `inquirer-autocomplete-prompt` doesn't seem to work with the way + // we're mocking prompt responses with `handleQuestions`. Instead, we're + // choosing the second template in the list, assuming it's the first one + // that contains a `package.json` (currently that's `apollo-graphql`). + t.is(await fs.fileExistsAsync(`${builder.directory}/test/functions/apollo-graphql/apollo-graphql.js`), true) + t.is(await fs.fileExistsAsync(`${builder.directory}/test/functions/apollo-graphql/package.json`), false) + t.is(typeof dependencies['apollo-server-lambda'], 'string') + + t.is(dependencies['@netlify/functions'], '^0.1.0') + }) + }) +}) + +test('should install function template dependencies in the function sub-directory if no site-level `package.json` is found', async (t) => { + const siteInfo = { + admin_url: 'https://app.netlify.com/sites/site-name/overview', + ssl_url: 'https://site-name.netlify.app/', + id: 'site_id', + name: 'site-name', + build_settings: { repo_url: 'https://github.com/owner/repo' }, + } + + const routes = [ + { + path: 'accounts', + response: [{ slug: 'test-account' }], + }, + { path: 'sites/site_id/service-instances', response: [] }, + { path: 'sites/site_id', response: siteInfo }, + { + path: 'sites', + response: [siteInfo], + }, + { path: 'sites/site_id', method: 'patch', response: {} }, + ] + + await withSiteBuilder('site-with-no-functions-dir-without-package-json', async (builder) => { + await builder.buildAsync() + + const createFunctionQuestions = [ + { + question: 'Enter the path, relative to your site', + answer: answerWithValue('test/functions'), + }, + { + question: 'Pick a template', + answer: answerWithValue(`${DOWN}${CONFIRM}`), + }, + { + question: 'name your function', + answer: answerWithValue(CONFIRM), + }, + ] + + await withMockApi(routes, async ({ apiUrl }) => { + const childProcess = execa(cliPath, ['functions:create'], { + env: { + NETLIFY_API_URL: apiUrl, + NETLIFY_SITE_ID: 'site_id', + NETLIFY_AUTH_TOKEN: 'fake-token', + }, + cwd: builder.directory, + }) + + handleQuestions(childProcess, createFunctionQuestions) + + await childProcess + + // NOTE: Ideally we should be running this test with a specific template, + // but `inquirer-autocomplete-prompt` doesn't seem to work with the way + // we're mocking prompt responses with `handleQuestions`. Instead, we're + // choosing the second template in the list, assuming it's the first one + // that contains a `package.json` (currently that's `apollo-graphql`). + t.is(await fs.fileExistsAsync(`${builder.directory}/test/functions/apollo-graphql/apollo-graphql.js`), true) + t.is(await fs.fileExistsAsync(`${builder.directory}/test/functions/apollo-graphql/package.json`), true) + }) + }) +}) + test('should not create a new function directory when one is found', async (t) => { const siteInfo = { admin_url: 'https://app.netlify.com/sites/site-name/overview',