feat!: use netlify/functions as the default functions directory #2188

eduardoboucas · 2021-01-20T11:20:06Z

Which problem is this pull request solving?

This PR makes netlify/functions the default functions directory.

List other issues or pull requests related to this problem

This closes #2016, https://github.com/netlify/product/issues/257, https://github.com/netlify/pod-the-builder/issues/91, and https://github.com/netlify/pod-the-builder/issues/89.

Additional details

Removed the warnings for the presence of the default functions directory
Added a test for checking that @netlify/config does not assign the default functions directory unless it exists

Checklist

Please add a x inside each checkbox:

I have read the contribution guidelines.
The status checks are successful (continuous integration). Those can be seen below.

eduardoboucas · 2021-01-20T12:26:05Z

packages/build/tests/plugins/fixtures/functions_src_dynamic/plugin.js

 const pRmdir = promisify(rmdir)

-const DEFAULT_FUNCTIONS_SRC = 'netlify-automatic-functions'
+const DEFAULT_FUNCTIONS_SRC_PARENT = 'netlify'


😖 This is super icky, but Node 8 does not support the recursive flag in fs.mkdir and this was the least terrible way of keeping the fixtures simple that I could think of. Any suggestions are more than welcome!

0c99646 is perhaps slightly less icky?

Looks good!
Another reason to drop Node 8.

ehmicky

This looks good, thanks!

When should we merge and release this @verythorough? How much heads up should we give users with the current warnings in the build logs?

Other note: this PR should also be followed by an update to @netlify/plugin-nextjs with the new netlify/functions. Otherwise, @netlify/plugin-nextjs would break.

Related: netlify/build#2188

eduardoboucas · 2021-01-21T10:09:53Z

Other note: this PR should also be followed by an update to @netlify/plugin-nextjs with the new netlify/functions. Otherwise, @netlify/plugin-nextjs would break.

Done in opennextjs/opennextjs-netlify#87.

…ctory

eduardoboucas · 2021-01-22T13:34:37Z

Resulting from the discussion in opennextjs/opennextjs-netlify#87, I've added backward-compatibility with netlify-automatic-functions in 5554f9d. It includes a warning message for when people are relying on this directory. The message probably needs some tweaking (@verythorough? 👋🏻 ), and ideally it'd be accompanied by a link to a community post and/or docs.

Current warning:

You or one of your plugins seem to be relying on a default directory for Netlify functions: "netlify-automatic-functions"
This behavior has been deprecated and our builds will stop automatically bundling functions from this directory.
We recommend renaming this directory to "netlify/functions", or explicitly setting "netlify-automatic-functions" as the functions directory in your build settings if you wish to continue using it.

🔔 @ehmicky

eduardoboucas · 2021-01-22T13:43:35Z

packages/build/src/core/constants.js

 }) {
-  await validateNetlifyDir(logs, buildDir)
+  // @todo Remove once we drop support for the legact default functions directory.
+  await checkForLegacyDefaultFunctionsDir(logs, buildDir)


Theoretically there's nothing stopping people from explicitcly defining netlify-automatic-functions in the config as being their functions directory, in which case this warning should not be printed. However, to achieve this we would need to move this logic to @netlify/config, because at this point we don't know whether functions was added by the user or by us.

@ehmicky do you have an opinion on this?

We ran some logs few weeks ago which confirmed that virtually no one was using netlify-automatic-functions since we have not documented it.

The main risk here is for users of @netlify/plugin-nextjs who installed it in their package.json. Those users will not know about netlify-automatic-functions since this is done under the hood. Instead, the way to fix this error would be for them to upgrade @netlify/plugin-nextjs (after we've released the new versions using netlify/functions).

I think this warning message is still safe to keep, but we might not need to move it to @netlify/config.

On the other hand, we might want to create an issue/PR to print a warning message in @netlify/build when a build is using the old version of @netlify/plugin-nextjs. What do you think?

ehmicky

This is looking great! 🎉

When releasing this, we should make sure to test this in production together with the current version of @netlify/plugin-nextjs (the one still using neltify/functions).

verythorough · 2021-01-27T04:10:19Z

When should we merge and release this @verythorough? How much heads up should we give users with the current warnings in the build logs?

In the related community update, we said February. As soon as dev and docs work is ready, we can release as early as next week.

verythorough · 2021-01-27T04:13:53Z

Oh, and pinging @rstavchansky for the copy review request. Thanks!

rstavchansky · 2021-01-27T15:29:29Z

@eduardoboucas Here's a suggested copy edit of the error. Feel free to adjust formatting as needed so it is readable in the output.

Detected site repository path: `{build_dir}/netlify-automatic-functions`. Netlify no longer recognizes this path as a default Functions directory location and can’t detect and build serverless functions stored there. To avoid potential build failures, either:
 - rename the Functions directory to `netlify/functions`
 - or explicitly set `netlify-automatic-functions` as the Functions directory in your site’s build settings.

Thank you for the ping @verythorough

eduardoboucas · 2021-01-27T18:53:05Z

Perfect, thanks @rstavchansky! I'm sorry I didn't ping you initially, I'm still learning who's responsible for what. ☹️

janosh · 2021-01-30T15:36:01Z

Perhaps you could fix the typo "non-exiting directory" at

build/packages/build/src/log/messages/core_commands.js

Line 7 in 399b32d

    
           log(logs, `The Netlify Functions setting targets a non-exiting directory: ${relativeFunctionsSrc}`)

as part of this PR.

ehmicky · 2021-02-01T14:09:30Z

The typo is fixed in #2236 (we should strive to keep PRs small and single-purposed).

rstavchansky

Thanks for the tweak to the error message about the plug-in upgrade. The wording looks fine to me. 👍

Related: netlify/build#2188

feat!: use netlify/functions as the default functions directory

fca4286

eduardoboucas requested a review from ehmicky January 20, 2021 11:20

github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jan 20, 2021

eduardoboucas added the needs docs label Jan 20, 2021

chore: fix tests in Node 8

0add8d9

eduardoboucas commented Jan 20, 2021

View reviewed changes

chore: clean up tests

0c99646

ehmicky approved these changes Jan 20, 2021

View reviewed changes

eduardoboucas added a commit to opennextjs/opennextjs-netlify that referenced this pull request Jan 21, 2021

Use new default functions directory

1b0cf55

Related: netlify/build#2188

eduardoboucas mentioned this pull request Jan 21, 2021

Use new default functions directory opennextjs/opennextjs-netlify#87

Merged

eduardoboucas added 2 commits January 22, 2021 11:23

Merge branch 'master' into feat/new-functions-directory

022cbef

feat: add backward-compatibility for netlify-automatic-functions dire…

5554f9d

…ctory

eduardoboucas requested a review from ehmicky January 22, 2021 13:37

eduardoboucas commented Jan 22, 2021

View reviewed changes

Merge branch 'master' into feat/new-functions-directory

ad918ae

ehmicky approved these changes Jan 22, 2021

View reviewed changes

eduardoboucas added 3 commits February 2, 2021 11:23

Merge branch 'master' into feat/new-functions-directory

a68da83

chore: update test snapshots

8b2dd8e

chore: update netlify-automatic-functions warning message

c8c403c

eduardoboucas mentioned this pull request Feb 4, 2021

Remove support for netlify-automatic-functions directory #2252

Closed

rstavchansky approved these changes Feb 4, 2021

View reviewed changes

Merge branch 'master' into feat/new-functions-directory

388758e

chore: update test snapshots

0db710d

eduardoboucas merged commit 84e1e07 into master Feb 4, 2021

This was referenced Feb 4, 2021

chore: release @netlify/build 9.0.0 #2253

Merged

chore: release @netlify/config 4.0.0 #2254

Merged

ehmicky deleted the feat/new-functions-directory branch February 4, 2021 16:39

ehmicky pushed a commit to opennextjs/opennextjs-netlify that referenced this pull request Mar 18, 2021

Use new default functions directory

bfad3f1

Related: netlify/build#2188

feat!: use netlify/functions as the default functions directory #2188

feat!: use netlify/functions as the default functions directory #2188

Uh oh!

Conversation

eduardoboucas commented Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eduardoboucas Jan 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eduardoboucas Jan 20, 2021

Choose a reason for hiding this comment

Uh oh!

ehmicky Jan 20, 2021

Choose a reason for hiding this comment

Uh oh!

ehmicky left a comment

Choose a reason for hiding this comment

Uh oh!

eduardoboucas commented Jan 21, 2021

Uh oh!

eduardoboucas commented Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eduardoboucas Jan 22, 2021

Choose a reason for hiding this comment

Uh oh!

ehmicky Jan 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ehmicky left a comment

Choose a reason for hiding this comment

Uh oh!

verythorough commented Jan 27, 2021

Uh oh!

verythorough commented Jan 27, 2021

Uh oh!

rstavchansky commented Jan 27, 2021

Uh oh!

eduardoboucas commented Jan 27, 2021

Uh oh!

janosh commented Jan 30, 2021

Uh oh!

ehmicky commented Feb 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rstavchansky left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

eduardoboucas commented Jan 20, 2021 •

edited

Loading

eduardoboucas Jan 20, 2021 •

edited

Loading

eduardoboucas commented Jan 22, 2021 •

edited

Loading

ehmicky Jan 22, 2021 •

edited

Loading

ehmicky commented Feb 1, 2021 •

edited

Loading