TS config cleanup - Part 2 #2407
Conversation
There was a problem hiding this comment.
These functions were extracted into a module with virtually no changes (the only change is one usage of the orElse function).
I wanted to move WaspFilePath, WaspLangFile, and others from Project.Common, but that created a circular dependency, so I kept them there.
I've only exported the symbols required by other modules:
findWaspFileandanalyzeWaspFilemust be exported as two distinct functions because ouranalyzefunction first looks for the file, does some other stuff (e.g., parsesschema.prisma), and only then comes back to the wasp file to analyze it.analyzeWaspFileContentis a lower-level function used by AI (because there's no file on disk).
There was a problem hiding this comment.
This file feels little busy to me, it feels like two different files.
I'd maybe go for Wasp.Project.WaspFile.DSL and Wasp.Project.WaspFile.Ts modules (names could be better). This way both files would be smaller and easier to follow IMHO.
There was a problem hiding this comment.
+1 to what @infomiho said, that also makes sense to me.
As for cyclic deps -> I guess we would have to figure out what is the best way to organize stuff. I am guessing the problem is that in other places of code we have stuff organized by "Analyze" and here you put all WaspFile stuff into one file including analysis and types and so on, so there are some cycles. Or it is something else. Anyway, maybe for example you could do it so that operations like analyze and compile and are under Project.WaspFile.Analyze. Or Project.Analyze.WaspFile if there is such file. I am making up stuff here, but there must be a way to arrange stuff so it all makes sense. But we don'th ave to do it now if you think that is too much, of course, it can stay like this if you think this is fine for now.
There was a problem hiding this comment.
I specifically didn't want to do this (I prefer a single file). We discussed it in the original PR too.
But since you both prefer two files, I'll change it.
As for the cyclic deps, it was much simpler and caused by the stuff in common (e.g., this file uses WaspProjectDir from Common, and Common would be using WaspLangFile from here) .
There was a problem hiding this comment.
I split it in the end.
| @@ -88,7 +88,6 @@ dotWaspInfoFileInGeneratedCodeDir = [relfile|.waspinfo|] | |||
| packageJsonInWaspProjectDir :: Path' (Rel WaspProjectDir) (File PackageJsonFile) | |||
| packageJsonInWaspProjectDir = [relfile|package.json|] | |||
|
|
|||
| -- TODO: The entire tsconfig story is very fragile | |||
There was a problem hiding this comment.
Did we cover this with some sort of an issue?
There was a problem hiding this comment.
Yes and no.
I wrote the comment back when the code looked differently and left it by accident. Still, there are things we could improve. And those are covered by other issues, such as:
- Figure out what to do with the
wasp ts-setupcommand for Wasp's TS config #2390 - Ensure config files in a Wasp project are set up correctly #2426
I created the second one now, so thanks for pointing this out :)
waspc/src/Wasp/Project/WaspFile.hs
Outdated
| Path' Abs (File WaspTsFile) -> | ||
| IO (Either [CompileError] [AS.Decl]) | ||
| analyzeWaspTsFile waspProjectDir prismaSchemaAst waspFilePath = runExceptT $ do | ||
| -- TODO: I'm not yet sure where tsconfig.node.json location should come from |
There was a problem hiding this comment.
| -- TODO: I'm not yet sure where tsconfig.node.json location should come from | |
| -- TODO: I'm not yet sure where tsconfig.wasp.json location should come from |
There was a problem hiding this comment.
This file feels little busy to me, it feels like two different files.
I'd maybe go for Wasp.Project.WaspFile.DSL and Wasp.Project.WaspFile.Ts modules (names could be better). This way both files would be smaller and easier to follow IMHO.
| @@ -250,6 +251,9 @@ eitherToMaybe :: Either e a -> Maybe a | |||
| eitherToMaybe (Right x) = Just x | |||
| eitherToMaybe (Left _) = Nothing | |||
|
|
|||
| orElse :: Maybe a -> a -> a | |||
| import Wasp.Util (indent, insertAt, leftPad) | ||
| import qualified Wasp.Util.Terminal as T | ||
|
|
||
| -- | Transforms compiler error (error with parse context) into an informative, pretty String that | ||
| -- can be printed directly into the terminal. It uses terminal features like escape codes | ||
| -- (colors, styling, ...). | ||
| showCompilerErrorForTerminal :: (Path' Abs (File f), String) -> (String, Ctx) -> String | ||
| showCompilerErrorForTerminal :: (Path' Abs (File WaspLangFile), String) -> (String, Ctx) -> String |
There was a problem hiding this comment.
+1 to what @infomiho said, that also makes sense to me.
As for cyclic deps -> I guess we would have to figure out what is the best way to organize stuff. I am guessing the problem is that in other places of code we have stuff organized by "Analyze" and here you put all WaspFile stuff into one file including analysis and types and so on, so there are some cycles. Or it is something else. Anyway, maybe for example you could do it so that operations like analyze and compile and are under Project.WaspFile.Analyze. Or Project.Analyze.WaspFile if there is such file. I am making up stuff here, but there must be a way to arrange stuff so it all makes sense. But we don'th ave to do it now if you think that is too much, of course, it can stay like this if you think this is fine for now.
Fixes the technical dept left over by #2276