focalboard icon indicating copy to clipboard operation
focalboard copied to clipboard

Tech Enhancement: Support Absolute Imports

Open Pinjasaur opened this issue 2 years ago • 2 comments

Summary

Support absolute imports over our current usage of relative imports i.e.

image

@asaadmahmood brought this up (thread linked below) w/ this supporting article: https://hackernoon.com/react-pro-tip-use-absolute-imports-for-better-readability-and-easy-refactoring-2ad5c7f2f957

How important this is to me and why

Importance: High/Medium/Low

Use cases: 1. 2. 3.

Additional context/similar features

Relevant (Staff-only) thread: https://community.mattermost.com/private-core/pl/sieq4acxgbrwxcfsf47h94hxor

Pinjasaur avatar Aug 22 '22 15:08 Pinjasaur

Does this need to be done all at once or can we do it piecemeal?

sbishel avatar Aug 22 '22 19:08 sbishel

Does this need to be done all at once or can we do it piecemeal?

@sbishel Good question—would have to experiment locally and see if it's the case or not. Not sure if this could be paired with linter config or not. 3/5, would rather have consistent imports than a mix of absolute & relative.

Pinjasaur avatar Aug 22 '22 19:08 Pinjasaur

Reviewing the article it looks like you're able to mix relative & absolute imports in the same file.

~~You can also create a jsconfig.json to pair with VS Code so autocomplete continues to work as expected e.g.~~ (We're already using tsconfig.json so it should all be fine and work with VS Code as expected)


Doing more searching, I found this article too: https://jeffchen.dev/posts/Automatically-Fixing-Relative-Imports-with-ESLint/

It mentions using TypeScript's baseUrl config key to do the same thing along with ESLint to --fix any relative imports into absolute imports. I noticed we're already using baseUrl in the repo and dug in a bit more and came up with this patch which seems to pass a basic smoketest i.e. it all builds:

diff --git a/mattermost-plugin/webapp/tsconfig.json b/mattermost-plugin/webapp/tsconfig.json
index 32c6a731..5eebbc15 100644
--- a/mattermost-plugin/webapp/tsconfig.json
+++ b/mattermost-plugin/webapp/tsconfig.json
@@ -14,7 +14,14 @@
 		"incremental": false,
 		"outDir": "./dist",
 		"moduleResolution": "node",
-        "typeRoots": [ "./src/types", "./node_modules/@types"]
+        "typeRoots": [ "./src/types", "./node_modules/@types"],
+		"baseUrl": ".",
+		"paths": {
+			"*": [
+				"*",
+				"../../webapp/src/*"
+			]
+		}
 	},
 	"include": [
         "src",
diff --git a/webapp/src/tsconfig.json b/webapp/src/tsconfig.json
index 54bb16f0..dfc9a4f0 100644
--- a/webapp/src/tsconfig.json
+++ b/webapp/src/tsconfig.json
@@ -4,6 +4,7 @@
 		"baseUrl": ".",
 		"paths": {
 			"*": [
+				"*",
 				"../node_modules/*",
 				"../@custom_types/*"
 			]
diff --git a/webapp/tsconfig.json b/webapp/tsconfig.json
index aa65defc..c8dd09ff 100644
--- a/webapp/tsconfig.json
+++ b/webapp/tsconfig.json
@@ -12,7 +12,7 @@
         "allowJs": true,
         "resolveJsonModule": true,
         "incremental": false,
-        "baseUrl": "src",
+        "baseUrl": "./src",
         "outDir": "./dist",
         "moduleResolution": "node"
     },

Leveraging this along with ESLint to enforce/--fix any relative -> absolute imports should put us in a good spot.

Pinjasaur avatar Sep 29 '22 20:09 Pinjasaur