Skip to content
This repository has been archived by the owner on May 10, 2021. It is now read-only.

add support for background functions in api pages only #171

Merged
merged 2 commits into from
Feb 25, 2021

Conversation

lindsaylevine
Copy link
Contributor

Fixes #166

want to get a range of eyes on this. there is a small risk mentioned in a comment inside regarding users possibly unknowingly creating background functions if theyre not familiar with them and happen to end their api page name in background. this was an ask by one specific user awhile ago and i think, as long as there's no real risk, this would be a great offering.

for now, i'm omitting an addition to the README documenting this support with a personal task to add it later. would like to run this by docs as well.

@lindsaylevine lindsaylevine added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Feb 19, 2021
Copy link
Collaborator

@FinnWoelm FinnWoelm left a comment

Choose a reason for hiding this comment

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

This looks great, @lindsaylevine! Very nice regex 😊

I agree with you that the risk of accidental background function seems rather minimal.

(One small caveat is that you can't have a background function with dynamic routing (e.g., api/[id] or api/[...params]). But I don't have a good idea/fix for this and I believe no one has asked for this, so seems totally OK for that to not be possible for the time being. Just wanted to flag that, because it crossed my mind.*)

*As a workaround, users can always create a dynamic API endpoint in addition to a non-dynamic background API endpoint and internally proxy from the former to the latter, converting path parameters into query parameters in the process (/:id to ?id=:id).

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

This looks good to me.
Per the risk we could add a summary - created x routes, y functions, z background functions or something similar (or event just print detect the following background functions) to make it more visible.

@sameerxanand
Copy link

Any ideas on when this might be published? Very useful and much needed feature!

@lindsaylevine
Copy link
Contributor Author

@sameeranand1 merging now, can release shortly !

@lindsaylevine lindsaylevine merged commit 3ad3b39 into main Feb 25, 2021
@lindsaylevine lindsaylevine deleted the ll/api-background-funcs branch February 25, 2021 03:23
lindsaylevine added a commit that referenced this pull request Feb 25, 2021
- add support for background functions in api pages only ([#171](#171))
- remove next as peer dependency ([#167](#167))
- add watch mode ([#162](#162))
@lindsaylevine
Copy link
Contributor Author

NoN was just released, individually. note though, if you're using the plugin, this will not be released immediately with the updated NoN version, unless necessary/requested.

@sameerxanand
Copy link

NoN was just released, individually. note though, if you're using the plugin, this will not be released immediately with the updated NoN version, unless necessary/requested.

Thanks! I’m using the node package - it’s in that release though, right? When you’re talking about the plug-in, you’re talking about through Netlify, right? Thanks!

@lindsaylevine
Copy link
Contributor Author

@sameeranand1 ah, yes, sorry for not being more specific! if you have next-on-netlify the package installed via npm or yarn, you're good to go now with v2.9.0. however, next-on-netlify will soon be fully absorbed by https://github.com/netlify/netlify-plugin-nextjs, which is basically the exact same as next-on-netlify, but is zero config/one-click installable at the moment and eventually will be auto-installed on all new next.js sites on netlify. to use that plugin, you'd follow the instructions in the plugin's README :). for now, though, you're free to keep using next-on-netlify as is!

@colinwirt
Copy link

Very nice regex 😊

Thanks @FinnWoelm! I look forward to contributing further to the project :)

Thanks for applying the change from my PR @lindsaylevine -- I updated my fork to require the path to include next_api_ after first adding an argument as you have done, but decided that was not necessary.

I'll add a few more tests and submit another PR.

Thanks again for adding the feature so quickly.

@lindsaylevine
Copy link
Contributor Author

lindsaylevine commented Feb 25, 2021

@colinwirt absolutely! and yes, i shouldve noted the regex was/is 100% the work of colin! looking forward to seeing what you contribute next :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow API routes to optionally run as background function
5 participants