Skip to content

Commit

Permalink
feat: install function template deps in site-level package.json (#3125
Browse files Browse the repository at this point in the history
)

* 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
  • Loading branch information
eduardoboucas authored Aug 10, 2021
1 parent 0b2bbfc commit af0e1bc
Show file tree
Hide file tree
Showing 2 changed files with 214 additions and 15 deletions.
81 changes: 67 additions & 14 deletions src/commands/functions/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'))
Expand All @@ -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}`)
}

Expand Down
148 changes: 147 additions & 1 deletion tests/command.functions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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',
Expand Down

1 comment on commit af0e1bc

@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

Package size: 331 MB

Please sign in to comment.