fix: outside import with relative module graph (#3417)

* refactor: removing duplicated `ImportMap` struct

* fix: resolving relative outside imports

It applies the import map walking to resolve the Module Graph

* refactor: revert some changes from 7a43b81

Moving `ImportMap` struct to `functions/pkg` and importing it inside
internals

* stamp: lint

* stamp: lint & using unix separator

* stamp: copying file buffer directly to writer

- we don't need to tee here, just copy from file to the writer will do.

* stamp: removing duplicated code for `ImportMap.Resolve()`

- using `afero.NewIOFS(fsys)` to cast to `io.FS`

* stamp: use absolute filepath while bundling

- Convert `srcPath` inside `addFile`, since docker mount always expect
an absolute path

* chore: bind relative imports in source files

* chore: refactor bind host modules

* chore: update unit tests

* fix: change import map flag to relative path

* chore: update unit tests

* chore: test for relative entrypoint

---------

Co-authored-by: Qiao Han <[email protected]>

Author: kallebysantos

internal/functions/deploy/bundle.go

@@ -122,41 +122,24 @@ func GetBindMounts(cwd, hostFuncDir, hostOutputDir, hostEntrypointPath, hostImpo
 			binds = append(binds, hostOutputDir+":"+dockerOutputDir+":rw")
 		}
 	}
-	// Allow entrypoints outside the functions directory
-	hostEntrypointDir := filepath.Dir(hostEntrypointPath)
-	if len(hostEntrypointDir) > 0 {
-		if !filepath.IsAbs(hostEntrypointDir) {
-			hostEntrypointDir = filepath.Join(cwd, hostEntrypointDir)
-		}
-		if !strings.HasSuffix(hostEntrypointDir, sep) {
-			hostEntrypointDir += sep
-		}
-		if !strings.HasPrefix(hostEntrypointDir, hostFuncDir) &&
-			!strings.HasPrefix(hostEntrypointDir, hostOutputDir) {
-			dockerEntrypointDir := utils.ToDockerPath(hostEntrypointDir)
-			binds = append(binds, hostEntrypointDir+":"+dockerEntrypointDir+":ro")
-		}
+	// Imports outside of ./supabase/functions will be bound by walking the entrypoint
+	modules, err := utils.BindHostModules(cwd, hostEntrypointPath, hostImportMapPath, fsys)
+	if err != nil {
+		return nil, err
 	}
-	// Imports outside of ./supabase/functions will be bound by absolute path
 	if len(hostImportMapPath) > 0 {
 		if !filepath.IsAbs(hostImportMapPath) {
 			hostImportMapPath = filepath.Join(cwd, hostImportMapPath)
 		}
-		importMap, err := utils.NewImportMap(hostImportMapPath, fsys)
-		if err != nil {
-			return nil, err
-		}
-		modules := importMap.BindHostModules()
 		dockerImportMapPath := utils.ToDockerPath(hostImportMapPath)
 		modules = append(modules, hostImportMapPath+":"+dockerImportMapPath+":ro")
-		// Remove any duplicate mount points
-		for _, mod := range modules {
-			hostPath := strings.Split(mod, ":")[0]
-			if !strings.HasPrefix(hostPath, hostFuncDir) &&
-				(len(hostOutputDir) == 0 || !strings.HasPrefix(hostPath, hostOutputDir)) &&
-				(len(hostEntrypointDir) == 0 || !strings.HasPrefix(hostPath, hostEntrypointDir)) {
-				binds = append(binds, mod)
-			}
+	}
+	// Remove any duplicate mount points
+	for _, mod := range modules {
+		hostPath := strings.Split(mod, ":")[0]
+		if !strings.HasPrefix(hostPath, hostFuncDir) &&
+			(len(hostOutputDir) == 0 || !strings.HasPrefix(hostPath, hostOutputDir)) {
+			binds = append(binds, mod)
 		}
 	}
 	return binds, nil

internal/functions/deploy/bundle_test.go

@@ -29,7 +29,7 @@ func TestDockerBundle(t *testing.T) {
 	t.Run("throws error on bundle failure", func(t *testing.T) {
 		// Setup in-memory fs
 		fsys := afero.NewMemMapFs()
-		absImportMap := filepath.Join(cwd, "hello", "deno.json")
+		absImportMap := filepath.Join("hello", "deno.json")
 		require.NoError(t, utils.WriteFile(absImportMap, []byte("{}"), fsys))
 		// Setup deno error
 		t.Setenv("TEST_DENO_ERROR", "bundle failed")

internal/functions/deploy/deploy.go

@@ -32,6 +32,19 @@ func Run(ctx context.Context, slugs []string, useDocker bool, noVerifyJWT *bool,
 	if len(slugs) == 0 {
 		return errors.Errorf("No Functions specified or found in %s", utils.Bold(utils.FunctionsDir))
 	}
+	// Flag import map is specified relative to current directory instead of workdir
+	cwd, err := os.Getwd()
+	if err != nil {
+		return errors.Errorf("failed to get working directory: %w", err)
+	}
+	if len(importMapPath) > 0 {
+		if !filepath.IsAbs(importMapPath) {
+			importMapPath = filepath.Join(utils.CurrentDirAbs, importMapPath)
+		}
+		if importMapPath, err = filepath.Rel(cwd, importMapPath); err != nil {
+			return errors.Errorf("failed to resolve relative path: %w", err)
+		}
+	}
 	functionConfig, err := GetFunctionConfig(slugs, importMapPath, noVerifyJWT, fsys)
 	if err != nil {
 		return err
@@ -83,10 +96,6 @@ func GetFunctionConfig(slugs []string, importMapPath string, noVerifyJWT *bool,
 	} else if err != nil {
 		return nil, errors.Errorf("failed to fallback import map: %w", err)
 	}
-	// Flag import map is specified relative to current directory instead of workdir
-	if len(importMapPath) > 0 && !filepath.IsAbs(importMapPath) {
-		importMapPath = filepath.Join(utils.CurrentDirAbs, importMapPath)
-	}
 	functionConfig := make(config.FunctionConfig, len(slugs))
 	for _, name := range slugs {
 		function, ok := utils.Config.Functions[name]

internal/functions/deploy/deploy_test.go

@@ -84,8 +84,7 @@ import_map = "./import_map.json"
 `)
 		require.NoError(t, err)
 		require.NoError(t, f.Close())
-		importMapPath, err := filepath.Abs(filepath.Join(utils.SupabaseDirPath, "import_map.json"))
-		require.NoError(t, err)
+		importMapPath := filepath.Join(utils.SupabaseDirPath, "import_map.json")
 		require.NoError(t, afero.WriteFile(fsys, importMapPath, []byte("{}"), 0644))
 		// Setup function entrypoint
 		entrypointPath := filepath.Join(utils.FunctionsDir, slug, "index.ts")

internal/functions/serve/serve.go

@@ -64,10 +64,8 @@ const (
 	dockerRuntimeInspectorPort = 8083
 )
 
-var (
-	//go:embed templates/main.ts
-	mainFuncEmbed string
-)
+//go:embed templates/main.ts
+var mainFuncEmbed string
 
 func Run(ctx context.Context, envFilePath string, noVerifyJWT *bool, importMapPath string, runtimeOption RuntimeOption, fsys afero.Fs) error {
 	// 1. Sanity checks.
@@ -121,6 +119,14 @@ func ServeFunctions(ctx context.Context, envFilePath string, noVerifyJWT *bool,
 	if err != nil {
 		return errors.Errorf("failed to get working directory: %w", err)
 	}
+	if len(importMapPath) > 0 {
+		if !filepath.IsAbs(importMapPath) {
+			importMapPath = filepath.Join(utils.CurrentDirAbs, importMapPath)
+		}
+		if importMapPath, err = filepath.Rel(cwd, importMapPath); err != nil {
+			return errors.Errorf("failed to resolve relative path: %w", err)
+		}
+	}
 	binds, functionsConfigString, err := populatePerFunctionConfigs(cwd, importMapPath, noVerifyJWT, fsys)
 	if err != nil {
 		return err

internal/functions/serve/serve_test.go

@@ -88,6 +88,7 @@ func TestServeCommand(t *testing.T) {
 	})
 
 	t.Run("throws error on missing import map", func(t *testing.T) {
+		utils.CurrentDirAbs = "/"
 		// Setup in-memory fs
 		fsys := afero.NewMemMapFs()
 		require.NoError(t, utils.InitConfig(utils.InitParams{ProjectId: "test"}, fsys))

internal/utils/deno.go

@@ -6,20 +6,20 @@ import (
 	"context"
 	"crypto/sha256"
 	"embed"
-	"encoding/json"
 	"fmt"
 	"io"
 	"io/fs"
 	"net/http"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"runtime"
 	"strings"
 
 	"github.com/go-errors/errors"
 	"github.com/spf13/afero"
-	"github.com/tidwall/jsonc"
+	"github.com/supabase/cli/pkg/function"
 )
 
 var (
@@ -197,7 +197,6 @@ func CopyDenoScripts(ctx context.Context, fsys afero.Fs) (*DenoScriptDir, error)
 
 		return nil
 	})
-
 	if err != nil {
 		return nil, err
 	}
@@ -210,87 +209,57 @@ func CopyDenoScripts(ctx context.Context, fsys afero.Fs) (*DenoScriptDir, error)
 	return &sd, nil
 }
 
-type ImportMap struct {
-	Imports map[string]string            `json:"imports"`
-	Scopes  map[string]map[string]string `json:"scopes"`
-}
-
-func NewImportMap(absJsonPath string, fsys afero.Fs) (*ImportMap, error) {
-	data, err := afero.ReadFile(fsys, absJsonPath)
+func newImportMap(relJsonPath string, fsys afero.Fs) (function.ImportMap, error) {
+	var result function.ImportMap
+	if len(relJsonPath) == 0 {
+		return result, nil
+	}
+	data, err := afero.ReadFile(fsys, relJsonPath)
 	if err != nil {
-		return nil, errors.Errorf("failed to load import map: %w", err)
+		return result, errors.Errorf("failed to load import map: %w", err)
 	}
-	result := ImportMap{}
 	if err := result.Parse(data); err != nil {
-		return nil, err
-	}
-	// Resolve all paths relative to current file
-	for k, v := range result.Imports {
-		result.Imports[k] = resolveHostPath(absJsonPath, v, fsys)
+		return result, err
 	}
-	for module, mapping := range result.Scopes {
-		for k, v := range mapping {
-			result.Scopes[module][k] = resolveHostPath(absJsonPath, v, fsys)
-		}
-	}
-	return &result, nil
-}
-
-func (m *ImportMap) Parse(data []byte) error {
-	data = jsonc.ToJSONInPlace(data)
-	decoder := json.NewDecoder(bytes.NewReader(data))
-	if err := decoder.Decode(&m); err != nil {
-		return errors.Errorf("failed to parse import map: %w", err)
+	unixPath := filepath.ToSlash(relJsonPath)
+	if err := result.Resolve(unixPath, afero.NewIOFS(fsys)); err != nil {
+		return result, err
 	}
-	return nil
+	return result, nil
 }
 
-func resolveHostPath(jsonPath, hostPath string, fsys afero.Fs) string {
-	// Leave absolute paths unchanged
-	if filepath.IsAbs(hostPath) {
-		return hostPath
+func BindHostModules(cwd, relEntrypointPath, relImportMapPath string, fsys afero.Fs) ([]string, error) {
+	importMap, err := newImportMap(relImportMapPath, fsys)
+	if err != nil {
+		return nil, err
 	}
-	resolved := filepath.Join(filepath.Dir(jsonPath), hostPath)
-	if exists, err := afero.Exists(fsys, resolved); !exists {
-		// Leave URLs unchanged
+	var modules []string
+	// Resolving all Import Graph
+	addModule := func(unixPath string, w io.Writer) error {
+		hostPath := filepath.FromSlash(unixPath)
+		if path.IsAbs(unixPath) {
+			hostPath = filepath.VolumeName(cwd) + hostPath
+		} else {
+			hostPath = filepath.Join(cwd, hostPath)
+		}
+		f, err := fsys.Open(hostPath)
 		if err != nil {
-			logger := GetDebugLogger()
-			fmt.Fprintln(logger, err)
+			return errors.Errorf("failed to read file: %w", err)
 		}
-		return hostPath
-	}
-	// Directory imports need to be suffixed with /
-	// Ref: https://deno.com/[email protected]/basics/import_maps
-	if strings.HasSuffix(hostPath, string(filepath.Separator)) {
-		resolved += string(filepath.Separator)
-	}
-	return resolved
-}
-
-func (m *ImportMap) BindHostModules() []string {
-	hostFuncDir, err := filepath.Abs(FunctionsDir)
-	if err != nil {
-		logger := GetDebugLogger()
-		fmt.Fprintln(logger, err)
-	}
-	binds := []string{}
-	for _, hostPath := range m.Imports {
-		if !filepath.IsAbs(hostPath) || strings.HasPrefix(hostPath, hostFuncDir) {
-			continue
+		defer f.Close()
+		if _, err := io.Copy(w, f); err != nil {
+			return errors.Errorf("failed to copy file content: %w", err)
 		}
 		dockerPath := ToDockerPath(hostPath)
-		binds = append(binds, hostPath+":"+dockerPath+":ro")
+		modules = append(modules, hostPath+":"+dockerPath+":ro")
+		return nil
 	}
-	for _, mapping := range m.Scopes {
-		for _, hostPath := range mapping {
-			if !filepath.IsAbs(hostPath) || strings.HasPrefix(hostPath, hostFuncDir) {
-				continue
-			}
-			dockerPath := ToDockerPath(hostPath)
-			binds = append(binds, hostPath+":"+dockerPath+":ro")
-		}
+	unixPath := filepath.ToSlash(relEntrypointPath)
+	if err := importMap.WalkImportPaths(unixPath, addModule); err != nil {
+		return nil, err
 	}
-	return binds
+	// TODO: support scopes
+	return modules, nil
 }
 
 func ToDockerPath(absHostPath string) string {

internal/utils/deno_test.go

@@ -31,7 +31,7 @@ func TestResolveImports(t *testing.T) {
 		require.NoError(t, fsys.Mkdir(filepath.Join(cwd, DbTestsDir), 0755))
 		require.NoError(t, fsys.Mkdir(filepath.Join(cwd, FunctionsDir, "child"), 0755))
 		// Run test
-		resolved, err := NewImportMap(jsonPath, fsys)
+		resolved, err := newImportMap(jsonPath, fsys)
 		// Check error
 		assert.NoError(t, err)
 		assert.Equal(t, "/tmp/", resolved.Imports["abs/"])
@@ -53,7 +53,7 @@ func TestResolveImports(t *testing.T) {
 		fsys := afero.NewMemMapFs()
 		require.NoError(t, afero.WriteFile(fsys, FallbackImportMapPath, importMap, 0644))
 		// Run test
-		resolved, err := NewImportMap(FallbackImportMapPath, fsys)
+		resolved, err := newImportMap(FallbackImportMapPath, fsys)
 		// Check error
 		assert.NoError(t, err)
 		assert.Equal(t, "https://deno.land", resolved.Scopes["my-scope"]["my-mod"])
@@ -62,37 +62,27 @@ func TestResolveImports(t *testing.T) {
 
 func TestBindModules(t *testing.T) {
 	t.Run("binds docker imports", func(t *testing.T) {
-		cwd, err := os.Getwd()
-		require.NoError(t, err)
-		importMap := ImportMap{
-			Imports: map[string]string{
-				"abs/":   "/tmp/",
-				"root":   cwd + "/common",
-				"parent": cwd + "/supabase/tests",
-				"child":  cwd + "/supabase/functions/child/",
-			},
-		}
+		fsys := afero.NewMemMapFs()
+		entrypoint := `import "https://deno.land"
+import "/tmp/index.ts"
+import "../common/index.ts"
+import "../../../supabase/tests/index.ts"
+import "./child/index.ts"`
+		require.NoError(t, WriteFile("/app/supabase/functions/hello/index.ts", []byte(entrypoint), fsys))
+		require.NoError(t, WriteFile("/tmp/index.ts", []byte{}, fsys))
+		require.NoError(t, WriteFile("/app/supabase/functions/common/index.ts", []byte{}, fsys))
+		require.NoError(t, WriteFile("/app/supabase/tests/index.ts", []byte{}, fsys))
+		require.NoError(t, WriteFile("/app/supabase/functions/hello/child/index.ts", []byte{}, fsys))
 		// Run test
-		mods := importMap.BindHostModules()
+		mods, err := BindHostModules("/app", "supabase/functions/hello/index.ts", "", fsys)
 		// Check error
+		assert.NoError(t, err)
 		assert.ElementsMatch(t, mods, []string{
-			"/tmp/:/tmp/:ro",
-			cwd + "/common:" + cwd + "/common:ro",
-			cwd + "/supabase/tests:" + cwd + "/supabase/tests:ro",
+			"/app/supabase/functions/hello/index.ts:/app/supabase/functions/hello/index.ts:ro",
+			"/tmp/index.ts:/tmp/index.ts:ro",
+			"/app/supabase/functions/common/index.ts:/app/supabase/functions/common/index.ts:ro",
+			"/app/supabase/tests/index.ts:/app/supabase/tests/index.ts:ro",
+			"/app/supabase/functions/hello/child/index.ts:/app/supabase/functions/hello/child/index.ts:ro",
 		})
 	})
-
-	t.Run("binds docker scopes", func(t *testing.T) {
-		importMap := ImportMap{
-			Scopes: map[string]map[string]string{
-				"my-scope": {
-					"my-mod": "https://deno.land",
-				},
-			},
-		}
-		// Run test
-		mods := importMap.BindHostModules()
-		// Check error
-		assert.Empty(t, mods)
-	})
 }

pkg/function/deploy.go

@@ -199,7 +199,7 @@ func writeForm(form *multipart.Writer, meta api.FunctionDeployMetadata, fsys fs.
 			return err
 		}
 	}
-	return walkImportPaths(meta.EntrypointPath, importMap, addFile)
+	return importMap.WalkImportPaths(meta.EntrypointPath, addFile)
 }
 
 type ImportMap struct {
@@ -254,7 +254,7 @@ func resolveHostPath(jsonPath, hostPath string, fsys fs.FS) string {
 // Ref: https://regex101.com/r/DfBdJA/1
 var importPathPattern = regexp.MustCompile(`(?i)(?:import|export)\s+(?:{[^{}]+}|.*?)\s*(?:from)?\s*['"](.*?)['"]|import\(\s*['"](.*?)['"]\)`)
 
-func walkImportPaths(srcPath string, importMap ImportMap, readFile func(curr string, w io.Writer) error) error {
+func (importMap *ImportMap) WalkImportPaths(srcPath string, readFile func(curr string, w io.Writer) error) error {
 	seen := map[string]struct{}{}
 	// DFS because it's more efficient to pop from end of array
 	q := make([]string, 1)
@@ -294,7 +294,8 @@ func walkImportPaths(srcPath string, importMap ImportMap, readFile func(curr str
 					substituted = true
 				}
 			}
-			// Ignore URLs and directories
+			// Ignore URLs and directories, assuming no sloppy imports
+			// https://github.com/denoland/deno/issues/2506#issuecomment-2727635545
 			if len(path.Ext(mod)) == 0 {
 				continue
 			}