antonk52 / cssmodules-language-server

autocompletion and go-to-defintion for cssmodules
MIT License
54 stars 8 forks source link

allow resolving in tsconfig even when no paths #16

Closed quentin-fox closed 11 months ago

quentin-fox commented 11 months ago

Background: in a repo I use, we have baseUrl: "." set in the tsconfig.json, but without any paths.

This allows us to import all of our files local to the project root, rather than relative imports throughout the project.

However, since we don't rely on the "paths" to alias our imports, then css module "Jump to Definition" functionality does not work, since it validates our tsconfig as having an invalid path.

This PR changes the validation logic so that it accepts null/undefined as a valid set of paths, since tsconfig works just fine when baseUrl is set, but no paths are set.

It also fixes an issue when validating the paths object, when it was in the shape of Record<string, string>, it would error out, as it tried to call String.prototype.every in the validate.tsconfigPaths, right after checking that !Array.isArray. This was slightly refactored, just to remove the ts-expect-error.

antonk52 commented 11 months ago

Hi and thanks for the PR. The issue you are running into is valid and should be fixed.

However, I disagree that TsconfigPaths can be null | undefined. Instead I suggest to remove baseUrl check and only check against paths in they are defined. Looks like baseUrl is optional to use with paths since TS 4.1. So I think it is okay to fallback to '.' in this case.

As of TypeScript 4.1, baseUrl is no longer required to be set when using paths.

This should produce a diff similar to this one which should also cover your case.

diff --git a/src/utils/resolveAliasedImport.ts b/src/utils/resolveAliasedImport.ts
index f0e471f..35a6c7b 100644
--- a/src/utils/resolveAliasedImport.ts
+++ b/src/utils/resolveAliasedImport.ts
@@ -63,34 +63,39 @@ export const resolveAliasedImport = ({
     const baseUrl = config.config?.compilerOptions?.baseUrl as void | string;
     const configLocation = path.dirname(config.filepath);

-    if (!validate.string(baseUrl)) {
-        return null;
-    }
-    if (!validate.tsconfigPaths(paths)) {
-        return null;
-    }
+    // if paths are defined in tsconfig try to resolve aliased import
+    if (validate.tsconfigPaths(paths)) {
+        for (const alias in paths) {
+            const aliasRe = new RegExp(alias.replace('*', '(.+)'), '');

-    for (const alias in paths) {
-        const aliasRe = new RegExp(alias.replace('*', '(.+)'), '');
+            const aliasMatch = importFilepath.match(aliasRe);

-        const aliasMatch = importFilepath.match(aliasRe);
+            if (aliasMatch == null) continue;

-        if (aliasMatch == null) continue;
+            for (const potentialAliasLocation of paths[alias]) {
+                const resolvedFileLocation = path.resolve(
+                    configLocation,
+                    baseUrl ?? '.',
+                    potentialAliasLocation
+                        // "./utils/*" -> "./utils/style.module.css"
+                        .replace('*', aliasMatch[1]),
+                );

-        for (const potentialAliasLocation of paths[alias]) {
-            const resolvedFileLocation = path.resolve(
-                configLocation,
-                baseUrl,
-                potentialAliasLocation
-                    // "./utils/*" -> "./utils/style.module.css"
-                    .replace('*', aliasMatch[1]),
-            );
+                if (!fs.existsSync(resolvedFileLocation)) continue;

-            if (!fs.existsSync(resolvedFileLocation)) continue;
-
-            return resolvedFileLocation;
+                return resolvedFileLocation;
+            }
         }
     }

+    // fallback to baseUrl
+    const resolvedFileLocation = path.resolve(
+        configLocation,
+        validate.string(baseUrl) ? baseUrl : '.',
+        importFilepath,
+    );
+
+    if (fs.existsSync(resolvedFileLocation)) return resolvedFileLocation;
+
     return null;
 };
quentin-fox commented 11 months ago

Sounds great to me! Will be good to also fix the baseUrl fallback behaviour if paths are defined.

I wasn't able to apply that patch file to the head of my fork, but was able to make the same changes in spirit.

quentin-fox commented 11 months ago

From the typescript docs:

"As of TypeScript 4.1, baseUrl is no longer required to be set when using paths."

Latest commit changes it so that there is no baseUrl default value if no paths are set - but if paths are set, then it uses the fallback of '.'