-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using TS SDK from waspc
#2276
base: main
Are you sure you want to change the base?
Using TS SDK from waspc
#2276
Changes from 17 commits
e228263
664eafe
1b75ab9
0c7b901
b7c4800
68bfe30
0758747
b7ecab4
e2e802e
2d27900
987243d
15e3e68
7600288
2b0c0ff
7ec9356
1597f4b
b887cd8
1bdc871
52378c4
115dce5
2617b6f
32275e5
b673308
3513b68
e9ef283
0dc7435
3e9efc8
24f65ec
edd7a85
ae76820
7eab0bc
8c8cb4f
d0ba9e5
4eec3c1
1699df2
5a6719e
3f48b98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file has a lot of diverse imports, indicating that it might want to be split into multiple files. I think we should split it probably -> we could certainly separate the logic that deals with WaspFile, and maybe then further split that into WaspLang and WaspTs logic, but not yet sure about that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought about extracting a But yeah, if you like it too, I'll do it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving this one for later. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,26 @@ | ||
module Wasp.Project.Analyze | ||
( analyzeWaspProject, | ||
readPackageJsonFile, | ||
analyzeWaspFileContent, | ||
findWaspFile, | ||
findPackageJsonFile, | ||
analyzePrismaSchema, | ||
WaspFile (..), | ||
) | ||
where | ||
|
||
import Control.Applicative ((<|>)) | ||
import Control.Arrow (ArrowChoice (left)) | ||
import Control.Concurrent (newChan) | ||
import Control.Concurrent.Async (concurrently) | ||
import Control.Monad.Except (ExceptT (..), runExceptT) | ||
import Control.Monad.IO.Class (liftIO) | ||
import qualified Data.Aeson as Aeson | ||
import Data.Conduit.Process.Typed (ExitCode (..)) | ||
import Data.List (find, isSuffixOf) | ||
import StrongPath (Abs, Dir, File', Path', toFilePath, (</>)) | ||
import StrongPath (Abs, Dir, File', Path', Rel, basename, toFilePath, (</>)) | ||
import qualified StrongPath as SP | ||
import StrongPath.TH (relfile) | ||
import StrongPath.Types (File) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see somebody went wild with accepting import suggestions from LS :D. You can just add those to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Heh. I have had this item on my Haskell list for quite some time. For some reason (and for some SP imports), LSP almost never suggests a top-level import from
This mental overhead drives me crazy, and sometimes I forget to change the imports manually later (like here). I would really like to solve this because I don't want to waste time manually editing the imports. @Martinsos @infomiho Is this specific to my setup, does it happen to you? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am in emacs, so it is a bit different, but first I get a suggestion for autocompletion: This one is correct, but when I accept, for some reason it doesn't also add an import automatically. So I get a compiler error that symbol is missing. I then tell LSP to suggest code actions, and then I get this Which is correct! I am offered also the lower level imports, but the first option I am offered is to add it to existing import list of StrongPath, and if I pick that, it adds import correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, glad to hear it's fixable because it's been annoying me for ages (particularly with StrongPath imports). I'll bother you in the office to help me figure it out. |
||
import qualified Wasp.Analyzer as Analyzer | ||
import Wasp.Analyzer.AnalyzeError (getErrorMessageAndCtx) | ||
import Wasp.Analyzer.Parser.Ctx (Ctx) | ||
|
@@ -23,10 +32,14 @@ import qualified Wasp.CompileOptions as CompileOptions | |
import qualified Wasp.ConfigFile as CF | ||
import Wasp.Error (showCompilerErrorForTerminal) | ||
import qualified Wasp.Generator.ConfigFile as G.CF | ||
import qualified Wasp.Generator.Job as J | ||
import Wasp.Generator.Job.IO (readJobMessagesAndPrintThemPrefixed) | ||
import Wasp.Generator.Job.Process (runNodeCommandAsJob) | ||
Comment on lines
+43
to
+45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is used here, then it sounds like it maybe shouldn't be in the Generator hm. What do you think? I didn't check the code though to see if there is anything Generator specific about it really or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right, nice catch. I didn't notice it was there. And there's nothing generator-specific about it. I think it's just there because the generator used to be the only one running jobs. I'll move it somewhere more appropriate. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving this one for later. |
||
import Wasp.Project.Common | ||
( CompileError, | ||
CompileWarning, | ||
WaspProjectDir, | ||
dotWaspDirInWaspProjectDir, | ||
findFileInWaspProjectDir, | ||
packageJsonInWaspProjectDir, | ||
prismaSchemaFileInWaspProjectDir, | ||
|
@@ -43,6 +56,7 @@ import Wasp.Psl.Valid (getValidDbSystemFromPrismaSchema) | |
import qualified Wasp.Psl.Valid as PslV | ||
import Wasp.Util (maybeToEither) | ||
import qualified Wasp.Util.IO as IOUtil | ||
import Wasp.Util.StrongPath (replaceRelExtension) | ||
import Wasp.Valid (ValidationError) | ||
import qualified Wasp.Valid as Valid | ||
|
||
|
@@ -60,7 +74,7 @@ analyzeWaspProject waspDir options = do | |
(Left prismaSchemaErrors, prismaSchemaWarnings) -> return (Left prismaSchemaErrors, prismaSchemaWarnings) | ||
-- NOTE: we are ignoring prismaSchemaWarnings if the schema was parsed successfully | ||
(Right prismaSchemaAst, _) -> | ||
analyzeWaspFile prismaSchemaAst waspFilePath >>= \case | ||
analyzeWaspFile waspDir prismaSchemaAst waspFilePath >>= \case | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I prefer keeping the |
||
Left errors -> return (Left errors, []) | ||
Right declarations -> | ||
analyzePackageJsonContent waspDir >>= \case | ||
|
@@ -69,8 +83,103 @@ analyzeWaspProject waspDir options = do | |
where | ||
fileNotFoundMessage = "Couldn't find the *.wasp file in the " ++ toFilePath waspDir ++ " directory" | ||
|
||
analyzeWaspFile :: Psl.Schema.Schema -> Path' Abs File' -> IO (Either [CompileError] [AS.Decl]) | ||
analyzeWaspFile prismaSchemaAst waspFilePath = do | ||
data WaspFile | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
= WaspLang !(Path' Abs (File WaspLangFile)) | ||
| WaspTs !(Path' Abs (File WaspTsFile)) | ||
|
||
data WaspLangFile | ||
|
||
data WaspTsFile | ||
Comment on lines
+96
to
+98
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if you went with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, can you elaborate how this works with the functions that use these types (compared to what we have now)? I looked into DataKinds, and I don't think there's use for them here, but I might be missing something. |
||
|
||
data CompiledWaspJsFile | ||
|
||
data SpecJsonFile | ||
|
||
analyzeWaspFile :: Path' Abs (Dir WaspProjectDir) -> Psl.Schema.Schema -> WaspFile -> IO (Either [CompileError] [AS.Decl]) | ||
analyzeWaspFile waspDir prismaSchemaAst = \case | ||
WaspLang waspFilePath -> analyzeWaspLangFile prismaSchemaAst waspFilePath | ||
WaspTs waspFilePath -> analyzeWaspTsFile waspDir prismaSchemaAst waspFilePath | ||
|
||
analyzeWaspTsFile :: Path' Abs (Dir WaspProjectDir) -> Psl.Schema.Schema -> 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 will come from | ||
-- because we also need that knowledge to generate a TS SDK project. | ||
compiledWaspJsFile <- ExceptT $ compileWaspTsFile waspProjectDir [relfile|tsconfig.node.json|] waspFilePath | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
specJsonFile <- ExceptT $ executeMainWaspJsFile waspProjectDir compiledWaspJsFile | ||
contents <- ExceptT $ readDeclsJsonFile specJsonFile | ||
liftIO $ putStrLn "Here are the contents of the spec file:" | ||
liftIO $ print contents | ||
return [] | ||
|
||
compileWaspTsFile :: | ||
Path' Abs (Dir WaspProjectDir) -> | ||
Path' (Rel WaspProjectDir) File' -> | ||
Path' Abs (File WaspTsFile) -> | ||
IO (Either [CompileError] (Path' Abs (File CompiledWaspJsFile))) | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
compileWaspTsFile waspProjectDir tsconfigNodeFileInWaspProjectDir waspFilePath = do | ||
chan <- newChan | ||
(_, tscExitCode) <- | ||
concurrently | ||
(readJobMessagesAndPrintThemPrefixed chan) | ||
( runNodeCommandAsJob | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
waspProjectDir | ||
"npx" | ||
[ "tsc", | ||
"-p", | ||
toFilePath (waspProjectDir </> tsconfigNodeFileInWaspProjectDir), | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"--noEmit", | ||
"false", | ||
"--outDir", | ||
toFilePath outDir | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function does its best to control The compiled
We want to control all of them, but it's a little tricky. Controlling the Controlling the file name is also simple. The compiler requires the file's name to be Controlling the
Note If we kept the contents of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't the file names The validation PR will help here since it enables to required certain values in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You can ignore that, it's a minor detail. I mostly use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems to me like you put effor tinto explaining the situation here, but I have to admit I still didn't get most of it. I am certainly missing some context + have less knowledge of the ts toolchain, but I still feel like I should have been able to understand it.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may be in too deep here. I think it's best if I explain this to you in person when you come back to the office (tomorrow or Wednesday), and you can help me write a comment that makes the situation clearer to the uninitiated. @infomiho Did you understand it? If so, can you help out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Martinsos I guess what Filip is trying to say by "emulate and control" - make sure that the compilation step happens in the same say the TS LSP interprets the This command runs Motivation: in order for us to know where the JS file will end up (so we can call it with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok tnx both -> @infomiho's explanation here helped + he explained it to me a bit more, so I think I get it now @sodic ! Here is what I understand, and how I would put it. Here, in this function, we are using That is it for that part, but there is another issue, which I think is a general issue, not specific to are we compiling with I actually like the idea of hardcoded name. We dont' need the flexibility + then we open the opportunity for multiple .wasp.ts files which we will want anyway, and you don't have to bother with validation or anything. We can just tell them it has to be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll leave this for after the release if that's alright @Martinsos. |
||
J.Wasp | ||
chan | ||
) | ||
case tscExitCode of | ||
ExitFailure _status -> return $ Left ["Error while running TypeScript compiler on the *.wasp.mts file."] | ||
ExitSuccess -> return $ Right absCompiledWaspJsFile | ||
where | ||
outDir = waspProjectDir </> dotWaspDirInWaspProjectDir | ||
absCompiledWaspJsFile = outDir </> compiledWaspJsFileInDotWaspDir | ||
compiledWaspJsFileInDotWaspDir = SP.castFile $ case replaceRelExtension (basename waspFilePath) ".mjs" of | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
Just path -> path | ||
Nothing -> error $ "Couldn't calculate the compiled JS file path for " ++ show waspFilePath | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I just go with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think providing an error message is nice, and yeah this casing here is boilerplatish, but better option than What is ugly is that error is not the last thing to read, although you could probably achieve that by doing a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weird that the LSP didn't suggest There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I know why the LSP didn't suggest it.
Should I leave it like this then? |
||
|
||
executeMainWaspJsFile :: Path' Abs (Dir WaspProjectDir) -> Path' Abs (File CompiledWaspJsFile) -> IO (Either [CompileError] (Path' Abs (File SpecJsonFile))) | ||
executeMainWaspJsFile waspProjectDir absCompiledMainWaspJsFile = do | ||
chan <- newChan | ||
(_, runExitCode) <- do | ||
concurrently | ||
(readJobMessagesAndPrintThemPrefixed chan) | ||
( runNodeCommandAsJob | ||
waspProjectDir | ||
"npx" | ||
-- TODO: Figure out how to keep running instructions in a single place | ||
-- (e.g., this is the same as the package name, but it's repeated in two places). | ||
-- Before this, I had the entrypoint file hardcoded, which was bad | ||
-- too: waspProjectDir </> [relfile|node_modules/wasp-config/dist/run.js|] | ||
[ "wasp-config", | ||
SP.fromAbsFile absCompiledMainWaspJsFile, | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SP.fromAbsFile absSpecOutputFile | ||
] | ||
J.Wasp | ||
chan | ||
) | ||
case runExitCode of | ||
ExitFailure _status -> return $ Left ["Error while running the compiled *.wasp.mts file."] | ||
ExitSuccess -> return $ Right absSpecOutputFile | ||
where | ||
-- TODO: The config part of the path is problematic because it relies on TSC to create it during compilation, | ||
-- see notes in compileWaspFile. | ||
absSpecOutputFile = waspProjectDir </> dotWaspDirInWaspProjectDir </> [relfile|spec.json|] | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
readDeclsJsonFile :: Path' Abs (File SpecJsonFile) -> IO (Either [CompileError] Aeson.Value) | ||
readDeclsJsonFile declsJsonFile = do | ||
byteString <- IOUtil.readFile declsJsonFile | ||
return $ Right $ Aeson.toJSON byteString | ||
|
||
analyzeWaspLangFile :: Psl.Schema.Schema -> Path' Abs (File WaspLangFile) -> IO (Either [CompileError] [AS.Decl]) | ||
analyzeWaspLangFile prismaSchemaAst waspFilePath = do | ||
waspFileContent <- IOUtil.readFile waspFilePath | ||
left (map $ showCompilerErrorForTerminal (waspFilePath, waspFileContent)) | ||
<$> analyzeWaspFileContent prismaSchemaAst waspFileContent | ||
|
@@ -118,15 +227,14 @@ constructAppSpec waspDir options packageJson parsedPrismaSchema decls = do | |
|
||
return $ runValidation ASV.validateAppSpec appSpec | ||
|
||
findWaspFile :: Path' Abs (Dir WaspProjectDir) -> IO (Maybe (Path' Abs File')) | ||
findWaspFile :: Path' Abs (Dir WaspProjectDir) -> IO (Maybe WaspFile) | ||
findWaspFile waspDir = do | ||
files <- fst <$> IOUtil.listDirectory waspDir | ||
return $ (waspDir </>) <$> find isWaspFile files | ||
return $ findWaspTsFile files <|> findWaspLangFile files | ||
sodic marked this conversation as resolved.
Show resolved
Hide resolved
|
||
where | ||
isWaspFile path = | ||
".wasp" | ||
`isSuffixOf` toFilePath path | ||
&& (length (toFilePath path) > length (".wasp" :: String)) | ||
findWaspTsFile files = WaspTs <$> findFileThatEndsWith ".wasp.mts" files | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I explained the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If that sticks, we will also wnat to have a comment in code somewhere that explains it. |
||
findWaspLangFile files = WaspLang <$> findFileThatEndsWith ".wasp" files | ||
findFileThatEndsWith suffix files = SP.castFile . (waspDir </>) <$> find ((suffix `isSuffixOf`) . toFilePath) files | ||
infomiho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
analyzePackageJsonContent :: Path' Abs (Dir WaspProjectDir) -> IO (Either [CompileError] PackageJson) | ||
analyzePackageJsonContent waspProjectDir = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have something like
data WaspFile
, so that we could doPath' Abs (File WaspFile)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch. We didn't before, but we'll have it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take care of this later. It requires dealing with cyclic dependencies.