feat(functions) update functions template to match new 'server' style#5063
feat(functions) update functions template to match new 'server' style#5063kallebysantos wants to merge 6 commits intosupabase:developfrom
Conversation
- Creating different templates for 'functions new' command - Handling '--auth' flag to specify which template use
Coverage Report for CI Build 24408017002Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage decreased (-0.06%) to 63.746%Details
Uncovered Changes
Coverage Regressions111 previously-covered lines in 8 files lost coverage.
Coverage Stats
💛 - Coveralls |
|
WIP: Do not merge until |
avallete
left a comment
avallete
left a comment
There was a problem hiding this comment.
A few comments. Otherwise LGTM.
| --header 'Content-Type: application/json' \ | ||
| --data '{"name":"Functions"}' | ||
|
|
||
| --header 'apiKey: {{ .PublishableKey }}' |
There was a problem hiding this comment.
issue
This wont execute if copy/pasted because of missing backslash
| --header 'apiKey: {{ .PublishableKey }}' | |
| --header 'apiKey: {{ .PublishableKey }}' \ |
|
|
||
| // Setup type definitions for built-in Supabase Runtime APIs | ||
| import "@supabase/functions-js/edge-runtime.d.ts" | ||
| import { withSupabase } from '@supabase/server' |
There was a problem hiding this comment.
nitpick
For code style consistency with the other templates it should be:
| import { withSupabase } from '@supabase/server' | |
| import { withSupabase } from "@supabase/server"; |
| "imports": { | ||
| "@supabase/functions-js": "jsr:@supabase/functions-js@^2" | ||
| "@supabase/functions-js": "jsr:@supabase/functions-js@^2", | ||
| "@supabase/server": "npm:@supabase/server" |
There was a problem hiding this comment.
question
Should this be pinned to a major version like for functions-js ?
| authMode := new_.AuthAccessModeApiKey | ||
| if len(authAccessMode.Value) > 0 { | ||
| authMode = new_.AuthAccessMode(authAccessMode.Value) | ||
| } |
There was a problem hiding this comment.
nitpick
Instead of manually setting this default could use the Value param in the flag definition like we do here: https://github.com/kallebysantos/supabase-cli/blob/6b96df41dec4f585ec3b0855ebdfc104af005e6e/cmd/gen.go#L64
authAccessMode = utils.EnumFlag{
Allowed: []string{
string(new_.AuthAccessModeAlways),
string(new_.AuthAccessModeApiKey),
string(new_.AuthAccessModeUser),
},
Value: string(new_.AuthAccessModeApiKey)
}
That'll also make --help show the real default automatically and let us drop the "(default apikey)" in the flag description: https://github.com/kallebysantos/supabase-cli/blob/6b96df41dec4f585ec3b0855ebdfc104af005e6e/cmd/functions.go#L183
4 participants
What kind of change does this PR introduce?
Feature
What is the new behavior?
Apply
withSupabasehelper from@supabase/serverAdditional context
A new optional flag
--authwas introduced for thefunctions newcommand, chose between the auth access template:alwaysNo credentials required - Open endpoints, wrappers that handle their own authapikeyValid publishable key or secret key - Use respectively for Client-facing, key-validated endpoints or Server-to-server, internal callsuserUser JWT required - Authenticated user endpointsCurrent default
--auth apikey--
Towards FUNC-563