Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow --unstable flag in functions serve #1139

Closed
wants to merge 2 commits into from

Conversation

sammccord
Copy link

What kind of change does this PR introduce?

feature

What is the current behavior?

Currently, supabase functions serve doesn't allow passing any custom deno flags into its underling deno run command. This change adds an --unstable flag.

fixes supabase/edge-runtime#210

What is the new behavior?

If --unstable flag is explicitly added to supabase functions serve --unstable, the flag is appended to the deno run string before the function path.

Screenshot 2023-05-23 at 9 27 41 AM

@sammccord sammccord requested a review from a team as a code owner May 23, 2023 15:29
@coveralls
Copy link

coveralls commented May 23, 2023

Pull Request Test Coverage Report for Build 5074472686

  • 2 of 6 (33.33%) changed or added relevant lines in 2 files are covered.
  • 8 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.09%) to 62.865%

Changes Missing Coverage Covered Lines Changed/Added Lines %
cmd/functions.go 1 2 50.0%
internal/functions/serve/serve.go 1 4 25.0%
Files with Coverage Reduction New Missed Lines %
internal/functions/serve/serve.go 1 57.18%
internal/gen/keys/keys.go 7 7.94%
Totals Coverage Status
Change from base Build 5071602015: -0.09%
Covered Lines: 4498
Relevant Lines: 7155

💛 - Coveralls

cmd/functions.go Show resolved Hide resolved
@@ -174,6 +174,11 @@ func Run(ctx context.Context, slug string, envFilePath string, noVerifyJWT *bool
} else {
return fmt.Errorf("failed to check index.ts for function %s: %w", slug, err)
}

if *unstable {
denoRunCmd = append(denoRunCmd, "--unstable")
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried deploying the puppeteer example with supabase functions deploy? If that works, we can always add the --unstable flag to deno run by default.

Ideally the same code should work in both local serving and production deployment.

Copy link
Author

@sammccord sammccord May 24, 2023

Choose a reason for hiding this comment

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

For probably unrelated reasons, the puppeteer example doesn't work locally all for me (even on official cli), there's some issue with readable streams that doesn't occur when running the same with deno run -

Error: channel closed
    at async UserWorker.fetch (ext:sb_user_workers/user_workers.js:54:21)
    at async Server.<anonymous> (file:///home/deno/main/index.ts:104:16)
    at async Server.#respond (https://deno.land/std@0.182.0/http/server.ts:220:24)
worker thread panicked Uncaught SyntaxError: The requested module '/v124/jszip@3.5.0/esnext/lib/readable-stream-browser.js' does not provide an export named 'default'
    at https://esm.sh/v124/jszip@3.5.0/esnext/jszip.mjs:2:291
worker "/home/deno/functions/puppeteer" returned an error: Uncaught SyntaxError: The requested module '/v124/jszip@3.5.0/esnext/lib/readable-stream-browser.js' does not provide an export named 'default'
    at https://esm.sh/v124/jszip@3.5.0/esnext/jszip.mjs:2:291

... but since it works great with deno deploy - https://gqlatxhpnxwuzspuqqvh.functions.supabase.co/puppeteer, and deno bundle and deno run both support --unstable, maybe it could be the default behavior? I just really want to play with Deno KV!

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm afraid Deno KV is not currently supported by supabase functions because we are switching to edge-runtime for both serving locally and prod deployment. The --unstable flag added in this PR targets the legacy deno relay server which we have deprecated.

Please consider opening a github issue with edge runtime repo to help our functions team prioritise Deno KV support.

@sweatybridge
Copy link
Contributor

I will close this PR for now until upstream support is added to edge runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Official Supabase Puppeteer example does not work locally
3 participants