fix: support main.ts as an entrypoint

[laktek]

What kind of change does this PR introduce?

Adds support for main.ts to be treated as a valid entrypoint along with index.ts. This allows functions generated with deno init to work without having to change the filename.

[coveralls]

Pull Request Test Coverage Report for Build 13515462009

Details

• 17 of 17 (100.0%) changed or added relevant lines in 2 files are covered.
• 5 unchanged lines in 1 file lost coverage.
• Overall coverage increased (+0.06%) to 58.296%

---

Files with Coverage Reduction	New Missed Lines	%
internal/gen/keys/keys.go	5	12.9%

Totals
Change from base Build 13513651492:	0.06%
Covered Lines:	7807
Relevant Lines:	13392

---

💛 - Coveralls

[avallete]

nitpick

I think we might want to extract that in a dedicated function as it seems to repeat here:

cli/pkg/config/config.go

Lines 656 to 658 in 0b8c968

	if len(function.Entrypoint) == 0 {
	function.Entrypoint = filepath.Join(builder.FunctionsDir, slug, "index.ts")
	} else if !filepath.IsAbs(function.Entrypoint) {

Also we probably need to change it there as well.

[laktek]

ah good catch!

[laktek]

Technically, it doesn't matter though. If the index.ts file doesn't exist, main.ts will be tried in GetFunctionConfig. We probably don't need to set a default in config. The same applies to import map paths, which we seem to repeat in config.go and GetFunctionConfig.

Is there another place where resolved config is used without calling GetFunctionConfig? If that's the case, GetFunctionConfig should either be in config or in pkg/utility instead of the deploy package.

cc: @sweatybridge

[sweatybridge]

GetFunctionConfig is mostly here for cli flag overrides, default import map, and auto detecting function slugs. They are not moved to pkg/config because the logic is either unnecessary or fragile for config based deployment.

To support main.ts as entrypoint, we should be updating pkg/config instead so it works for both branching and cli.

[laktek]

For now, I followed the same as import map paths and copied over the same code to pkg/config. 5af8b8a

I couldn't really figure out a clean way to abstract this logic. We should do a refactor for both entrypoint and import maps in a separate PR. Currently, config only handles explicitly defined function slugs, we should make it implicit (ie. go through function directories and then config.toml). Then we can resolve the function config once and reuse everywhere.

[sweatybridge]

I'd say keep only your changes to pkg/config.go and remove all changes to deploy.go.

If users run functions deploy without declaring anything in config.toml, only index.ts is going to be used, exactly the same as today. But if they declare [functions.some-slug], then main.ts can be used as fallback.

This way, we avoid introducing more implicit behaviour which makes maintenance difficult. And we nudge users towards best practices for CI/CD by declaring their functions in config.toml.

[laktek]

But if they declare [functions.some-slug], then main.ts can be used as a fallback.

Sadly, this would mean users must do another step after deno init --serve before they can test their functions. If users are going to do another step, it doesn't even matter to add support to main.ts. We can simply ask them to rename the file to index.ts instead.

The ideal DX I'm aiming for:

• deno init --serve supabase/functions/hello-world (or copy an existing project to supabase/functions)
• npx supabase function serve
• visit http://localhost:54321/functions/v1/hello-world and see the response

NOT having to manually edit config.toml or rename files/directories is important so users can get functions to work on the first attempt.

[sweatybridge]

May be it's best to discuss this offline.

Why is it important to support deno init --serve when we have supabase functions new? If the primary concern is portability, I think it's better to have a dedicated command for migrating deno scripts to supabase functions. For eg. https://x.com/polar_sh/status/1859912074115363227

This way we have full control over the DX rather than mixing deno and edge functions.

[sweatybridge]

Another option is allow specifying the full entrypoint path when serving / deploying, exactly like deno.

[sweatybridge]

A wildcard match here could open a can of worms with existing CI/CD pipelines as shared modules may be unintentionally deployed. I'd rather let users opt-in by declaring in config. For eg. if they declare [functions.some-slug] in config.toml, we can automatically check for both index.ts followed by main.ts.

[laktek]

hmm..don't we already have a check to exclude shared modules?

cli/internal/functions/deploy/deploy.go

Line 64 in cf4d803

if utils.FuncSlugPattern.MatchString(slug) {

(we already do a wildcard match on the directory, I don't think this makes it worse than that)

If there's a Glob to explicitly match both index.ts and main.ts, I'm happy to change that (tried, but I couldn't get it to work).

The problem with having [functions.some-slug] in config.toml is users have to add it manually. And if they do deno init --serve supabase/functions/my-func and then try to access the function it wouldn't work. If we can auto-populate the valid function slugs, it would improve the DX a lot.

[sweatybridge]

(we already do a wildcard match on the directory, I don't think this makes it worse than that)

The existing wildcard match does NOT eliminate shared modules like functions/_shared/index.ts. It will deploy a function named _shared.

Unfortunately this isn't something we can easily change now without risking breaking user's CI/CD pipeline. Because users may very well have a function named _something and expects it to be deployed.

IMO, the root of this evil is due to the implicit behaviour of finding entrypoints by glob patterns. It was ok for small scale local development, but not acceptable for CI/CD when you have 100s of functions to manage.

And if they do deno init --serve supabase/functions/my-func and then try to access the function it wouldn't work.

I suspect you've misunderstood my suggestion. I'm saying to parse main.ts as fallback entrypoint in pkg/config, not to completely ignore it. That way, both deno init --serve and functions deploy work.

[sweatybridge]

Suggested change

	indexEntrypoint := filepath.Join(builder.FunctionsDir, slug, "index.ts")
	mainEntrypoint := filepath.Join(builder.FunctionsDir, slug, "main.ts")
	if _, err := fs.Stat(fsys, indexEntrypoint); err == nil {
	function.Entrypoint = indexEntrypoint
	} else if _, err := fs.Stat(fsys, mainEntrypoint); err == nil {
	function.Entrypoint = mainEntrypoint
	}
	function.Entrypoint = filepath.Join(builder.FunctionsDir, slug, "index.ts")
	if _, err := fs.Stat(fsys, function.Entrypoint); errors.Is(err, os.ErrNotExist) {
	mainEntrypoint := filepath.Join(builder.FunctionsDir, slug, "main.ts")
	if _, err := fs.Stat(fsys, mainEntrypoint); err == nil {
	function.Entrypoint = mainEntrypoint
	}
	}

We should still pass down default entrypoint path.

[sweatybridge]

Suggested change

	indexEntrypoint := filepath.Join(functionDir, "index.ts")
	mainEntrypoint := filepath.Join(functionDir, "main.ts")
	if _, err := fsys.Stat(indexEntrypoint); err == nil {
	function.Entrypoint = indexEntrypoint
	} else if _, err := fsys.Stat(mainEntrypoint); err == nil {
	function.Entrypoint = mainEntrypoint
	} else {
	return nil, errors.Errorf("Cannot find a valid entrypoint file (index.ts or main.ts) for the '%s' function. Set the custom entrypoint path in config.toml", name)
	}
	function.Entrypoint = filepath.Join(functionDir, "index.ts")
	if _, err := fs.Stat(fsys, function.Entrypoint); errors.Is(err, os.ErrNotExist) {
	mainEntrypoint := filepath.Join(functionDir, "main.ts")
	if _, err := fs.Stat(fsys, mainEntrypoint); err == nil {
	function.Entrypoint = mainEntrypoint
	}
	}

ditto

One way to reduce duplication is to add a function.ResolvePaths method. I can work on that separately.

[laktek]

Sync'd offline to come up with an alternate workflow. Closing this PR.