Disable esparse's TS option to avoid removal of unused imports

internal/backends/nodejs/grab.go

@@ -1,15 +1,16 @@
 package nodejs
 
 import (
+	"io/ioutil"
+	"log"
+	"path/filepath"
+	"strings"
+
 	"github.com/amasad/esparse/ast"
 	"github.com/amasad/esparse/logging"
 	"github.com/amasad/esparse/parser"
 	"github.com/replit/upm/internal/api"
 	"github.com/replit/upm/internal/util"
-	"io/ioutil"
-	"log"
-	"path/filepath"
-	"strings"
 )
 
 var internalModules = []string{
@@ -70,17 +71,17 @@ type parseResult struct {
 
 func parseFile(source logging.Source, results chan parseResult) {
 	parseOptions := parser.ParseOptions{
-		IsBundling: true,
+		IsBundling:   true,
+		MangleSyntax: false,
 	}
 
 	// Always parse jsx
 	parseOptions.JSX.Parse = true
+	// TS parsing strips unused imports, that becomes
+	// inconsistent with the regex-based import searching used to
+	// generate the guess hash, so we disable it
+	parseOptions.TS.Parse = false
 
-	ext := getExt(source.AbsolutePath)
-
-	if ext == ".ts" || ext == ".tsx" {
-		parseOptions.TS.Parse = true
-	}
 	logo, _ := logging.NewDeferLog()
 
 	ast, ok := parser.Parse(logo, source, parseOptions)

internal/backends/nodejs/nodejs_test.go

@@ -8,13 +8,15 @@ import (
 	"github.com/replit/upm/internal/api"
 )
 
+type TestCase struct {
+	scenario    string
+	backend     api.LanguageBackend
+	fileContent string
+	expected    map[api.PkgName]bool
+}
+
 func TestNodejsYarnBackend_Guess(t *testing.T) {
-	tcs := []struct {
-		scenario    string
-		backend     api.LanguageBackend
-		fileContent string
-		expected    map[api.PkgName]bool
-	}{
+	tcs := []TestCase{
 		{
 			scenario: "Returns the imports when given a JavaScript file with normal imports",
 			backend:  NodejsNPMBackend,
@@ -27,6 +29,16 @@ func TestNodejsYarnBackend_Guess(t *testing.T) {
 				"react": true,
 			},
 		},
+		{
+			scenario: "Returns normal requires",
+			backend:  NodejsNPMBackend,
+			fileContent: `
+			const request = require('request');
+		`,
+			expected: map[api.PkgName]bool{
+				"request": true,
+			},
+		},
 		{
 			scenario: "Ignore internal imports",
 			backend:  NodejsNPMBackend,
@@ -35,6 +47,18 @@ func TestNodejsYarnBackend_Guess(t *testing.T) {
 		`,
 			expected: map[api.PkgName]bool{},
 		},
+		{
+			scenario: "Returns both requires and imports in mixed file",
+			backend:  NodejsNPMBackend,
+			fileContent: `
+			const request = require('request');
+			import yargs from 'yargs';
+		`,
+			expected: map[api.PkgName]bool{
+				"request": true,
+				"yargs":   true,
+			},
+		},
 		{
 			scenario: "Ignore local file imports",
 			backend:  NodejsNPMBackend,
@@ -73,41 +97,100 @@ func TestNodejsYarnBackend_Guess(t *testing.T) {
 				"@material-ui/core": true,
 			},
 		},
-	}
-
-	for _, tc := range tcs {
-		tc := tc
-		t.Run(tc.scenario, func(t *testing.T) {
-			dir, err := ioutil.TempDir(".", "temp")
-			if err != nil {
-				t.Error(err)
+		{
+			scenario: "Conditional require",
+			backend:  NodejsNPMBackend,
+			fileContent: `
+			if (process.env.NODE_ENV === "production") { 
+				require("node-fetch");
 			}
-			defer os.RemoveAll(dir)
+		`,
+			expected: map[api.PkgName]bool{
+				"node-fetch": true,
+			},
+		},
+		{
+			scenario: "types then conditional require",
+			backend:  NodejsNPMBackend,
+			fileContent: `
+			type Field<T> = { field: string; };
 
-			file, err := ioutil.TempFile(dir, "*.js")
-			if err != nil {
-				t.Error(err)
+			if (process.env.NODE_ENV === "production") { 
+				require("node-fetch");
 			}
+		`,
+			expected: map[api.PkgName]bool{},
+		},
+		{
+			scenario: "dynamic import",
+			backend:  NodejsNPMBackend,
+			fileContent: `
 
-			_, err = file.WriteString(tc.fileContent)
-			if err != nil {
-				t.Error(err)
+			if (process.env.NODE_ENV === "production") { 
+				import("node-fetch");
 			}
+		`,
+			expected: map[api.PkgName]bool{
+				"node-fetch": true,
+			},
+		},
+		{
+			scenario: "typings then dynamic import",
+			backend:  NodejsNPMBackend,
+			fileContent: `
+			type Field<T> = { field: string; };
 
-			result, ok := tc.backend.Guess()
-			if !ok {
-				t.Errorf("Guess return a non true value")
+			if (process.env.NODE_ENV === "production") { 
+				import("node-fetch");
 			}
+		`,
+			expected: map[api.PkgName]bool{},
+		},
+	}
 
-			if len(tc.expected) != len(result) {
-				t.Errorf("Expected length of result to match length of expected")
-			}
+	for _, tc := range tcs {
+		tc := tc
+		t.Run(tc.scenario+"in js", func(t *testing.T) {
+			verify(t, tc, "js")
 
-			for key := range tc.expected {
-				if _, ok := result[key]; !ok {
-					t.Errorf("Key %s not found in result map", key)
-				}
-			}
 		})
+
+		t.Run(tc.scenario+" in ts", func(t *testing.T) {
+			verify(t, tc, "ts")
+
+		})
+	}
+}
+
+func verify(t *testing.T, tc TestCase, extension string) {
+	dir, err := ioutil.TempDir(".", "temp")
+	if err != nil {
+		t.Error(err)
+	}
+	defer os.RemoveAll(dir)
+
+	file, err := ioutil.TempFile(dir, "*."+extension)
+	if err != nil {
+		t.Error(err)
+	}
+
+	_, err = file.WriteString(tc.fileContent)
+	if err != nil {
+		t.Error(err)
+	}
+
+	result, ok := tc.backend.Guess()
+	if !ok {
+		t.Errorf("Guess return a non true value")
+	}
+
+	if len(tc.expected) != len(result) {
+		t.Errorf("Expected length of result to match length of expected")
+	}
+
+	for key := range tc.expected {
+		if _, ok := result[key]; !ok {
+			t.Errorf("Key %s not found in result map", key)
+		}
 	}
 }