Skip to content
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

Small fixes in the generator #2269

Merged
merged 12 commits into from
Sep 27, 2024
7 changes: 1 addition & 6 deletions waspc/src/Wasp/AppSpec/Job.hs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ data Perform = Perform
}
deriving (Show, Eq, Data)

instance IsDecl Perform

-- Allows jobs to run via some cron schedule.
data Schedule = Schedule
{ cron :: String, -- 5 field cron expression, exe: "*/5 * * * *".
Expand All @@ -49,13 +47,10 @@ data Schedule = Schedule
}
deriving (Show, Eq, Data)

instance IsDecl Schedule

-- These are optional executor-specific JSON options we pass
-- directly through to the executor when submitting jobs.
data ExecutorOptions = ExecutorOptions
{ pgBoss :: Maybe JSON,
simple :: Maybe JSON
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't use this as far as I can see, and it's not documented.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Origin story:
Screenshot 2024-09-16 at 15 09 21

{ pgBoss :: Maybe JSON
}
deriving (Show, Eq, Data)

Expand Down
2 changes: 1 addition & 1 deletion waspc/src/Wasp/Generator/DbGenerator.hs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ generatePrismaClient spec projectRootDir = do
generatePrismaClientIfEntitiesExist :: IO (Maybe GeneratorError)
generatePrismaClientIfEntitiesExist
| entitiesExist =
either (Just . GenericGeneratorError) (const Nothing) <$> DbOps.generatePrismaClient projectRootDir
either (Just . GenericGeneratorError) (const Nothing) <$> DbOps.generatePrismaClient projectRootDir
| otherwise = return Nothing

entitiesExist = not . null $ getEntities spec
Expand Down
4 changes: 2 additions & 2 deletions waspc/src/Wasp/Generator/Setup.hs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ runSetup spec projectRootDir sendMessage = do
setUpDatabaseResults@(_warnings, _errors@[]) -> do
-- todo(filip): Should we consider building SDK as part of code generation?
-- todo(filip): Avoid building on each setup if we don't need to.
buildsSdkResults <- buildSdk projectRootDir sendMessage
return $ setUpDatabaseResults <> buildsSdkResults
buildSdkResults <- buildSdk projectRootDir sendMessage
return $ setUpDatabaseResults <> buildSdkResults
setUpDatabaseResults -> return setUpDatabaseResults
Left npmInstallError -> return ([], [npmInstallError])

Expand Down
87 changes: 80 additions & 7 deletions waspc/src/Wasp/Project/Analyze.hs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,24 @@ module Wasp.Project.Analyze
findWaspFile,
findPackageJsonFile,
analyzePrismaSchema,
WaspFile (..),
)
where

import Control.Applicative ((<|>))
import Control.Arrow (ArrowChoice (left))
import Control.Concurrent (newChan, readChan)
import Control.Concurrent.Async (concurrently)
import Control.Concurrent.Chan (Chan)
import Control.Monad.IO.Class (liftIO)
import qualified Data.Aeson as Aeson
import qualified Data.Conduit.Combinators as T
import Data.Conduit.Process.Typed (ExitCode (..))
import Data.List (find, isSuffixOf)
import qualified Data.Text as T
import StrongPath (Abs, Dir, File', Path', toFilePath, (</>))
import qualified StrongPath as SP
import StrongPath.Types (File)
import qualified Wasp.Analyzer as Analyzer
import Wasp.Analyzer.AnalyzeError (getErrorMessageAndCtx)
import Wasp.Analyzer.Parser.Ctx (Ctx)
Expand All @@ -23,6 +34,10 @@ 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.IO.PrefixedWriter (printJobMessagePrefixed, runPrefixedWriter)
import Wasp.Generator.Job.Process (runNodeCommandAsJob)
import Wasp.Project.Common
( CompileError,
CompileWarning,
Expand Down Expand Up @@ -69,8 +84,57 @@ 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
analyzeWaspFile :: Psl.Schema.Schema -> WaspFile -> IO (Either [CompileError] [AS.Decl])
analyzeWaspFile prismaSchemaAst = \case
WaspLangFile waspFilePath -> analyzeWaspLangFile prismaSchemaAst waspFilePath
WaspTsFile waspFilePath -> analyzeWaspTsFile prismaSchemaAst waspFilePath

analyzeWaspTsFile :: Psl.Schema.Schema -> Path' Abs File' -> IO (Either [CompileError] [AS.Decl])
analyzeWaspTsFile _prismaSchemaAst waspFilePath = do
let workingDir = SP.parent waspFilePath
chan <- newChan
(_, tscExitCode) <-
concurrently
(readJobMessagesAndPrintThemPrefixed chan)
(runNodeCommandAsJob workingDir "npx" ["tsc", "-p", "tsconfig.node.json"] J.Wasp chan)

case tscExitCode of
ExitFailure _status -> return $ Left ["Error while running TypeScript compiler on the *.wasp.mts file."]
ExitSuccess -> do
otherChan <- newChan
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow otherChan, what happened here :D

(_, runExitCode) <-
concurrently
(handleRunJsMessages otherChan)
( runNodeCommandAsJob
workingDir
"node"
[ SP.fromAbsDir workingDir ++ "node_modules/wasp-ts-sdk/dist/run.js",
SP.fromAbsDir workingDir ++ ".wasp/config/main.wasp.mjs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work on Win, because workingDir will have Win separators and you have posix separators on the right.
In general I recommend converting from StrongPath at the last moment possible, that will save you from mistakes like this.
So in this case:

SP.fromAbsFile $ workingDir </> [relfile|node_modules/wasp-ts-sdk/dist/run.js|],
...

]
J.Wasp
otherChan
)
case runExitCode of
ExitFailure _status -> return $ Left ["Error while running the compiled *.wasp.mts file."]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use actual file name here. You have it available. And you avoid a bit of hardcoding.

ExitSuccess -> return $ Right []
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No decls? Aha I guess we don't yet have that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where
handleRunJsMessages :: Chan J.JobMessage -> IO ()
handleRunJsMessages = runPrefixedWriter . processMessages
processMessages chan = do
jobMsg <- liftIO $ readChan chan
case J._data jobMsg of
J.JobOutput payload J.Stdout -> do
-- let payload:: String = read $ T.unpack text
liftIO $ putStrLn "Ovo smo dobili na stdout"
-- parse payload as json
let json = Aeson.toJSON payload
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha this is still WIP! Sorry I thought this PR is ready for review since it wasn't draft PR.

Copy link
Contributor Author

@sodic sodic Sep 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aw man, I made a git error.
The Analyze.hs file was never supposed to be here, I must have committed it by accident.

Sorry for wasting your time, my bad!

What worries me is that you got through the entire review without noticing that the code is too horrible for something I'd send to code review.

Commenting with a straight face at all the atrocities and only realizing something was off when you encountered a string in Croatian.

Is that what you think of me, Martin? I don't know whether to praise your patience or worry about your perception 😄

Anyway, I'll remove this file from the PR. Please take a look at the other files. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, I believed in you!

All right, I will take another look. Maybe of some the change syou did in this file though, and that I commented on, are still present in the version of the code you are woroking on? In that case do take a look at my comments on Analyze.hs and see if you can apply them.

Copy link
Member

@Martinsos Martinsos Sep 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe this is just your sneaky way of giving up on a bad PR :D? "ha it was a joke from the very start"

liftIO $ print json
return ()
J.JobOutput _ J.Stderr -> printJobMessagePrefixed jobMsg >> processMessages chan
J.JobExit {} -> return ()

analyzeWaspLangFile :: Psl.Schema.Schema -> Path' Abs File' -> IO (Either [CompileError] [AS.Decl])
analyzeWaspLangFile prismaSchemaAst waspFilePath = do
waspFileContent <- IOUtil.readFile waspFilePath
left (map $ showCompilerErrorForTerminal (waspFilePath, waspFileContent))
<$> analyzeWaspFileContent prismaSchemaAst waspFileContent
Expand Down Expand Up @@ -118,16 +182,25 @@ constructAppSpec waspDir options packageJson parsedPrismaSchema decls = do

return $ runValidation ASV.validateAppSpec appSpec

findWaspFile :: Path' Abs (Dir WaspProjectDir) -> IO (Maybe (Path' Abs File'))
data WaspFile
= WaspLangFile {_path :: !(Path' Abs File')}
| WaspTsFile {_path :: !(Path' Abs File')}
deriving (Show)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, feels a bit unusual. I get why you did it though, you want to be able to say it is one of them but you don't want to yet know which one. Records in sum types are usually an anti-pattern, but these records are identical except for the data constructor, so it works.

What if you want to specify somewhere that a function needs specifically path to WaspTS file? Or Sepcifically WaspLangFile? We have that knowledge but we didn't put it in StrongPath.

I guess soltuion would be

data WaspLangFile
data WaspTsFile
data WaspFile = WaspLang (Path' Abs WaspLangFile) | WaspTs (Path' Abs WaspTsFile)

And I wouldn't bother with _path, I probably won't be needing that anyway as I will most likely always want to deconstruct hm.
But ok, this is verbose a bit, and it is all quite localized so I guess it is not important. Although you do export WaspFile (..), which makes it less localized hm. Ok, I am ok with this what you have, although I would be tempted to go this extra step.


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
where
isWaspFile path =
".wasp"
`isSuffixOf` toFilePath path
findWaspTsFile files = WaspTsFile . (waspDir </>) <$> find (`hasExtension` ".wasp.mts") files
findWaspLangFile files = WaspLangFile . (waspDir </>) <$> find isWaspLangFile files
isWaspLangFile path =
path `hasExtension` ".wasp"
&& (length (toFilePath path) > length (".wasp" :: String))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm this is an interesting line of code -> we are making sure we don't pick up .wasp dir as wasp file.
When I saw this, I felt like we should also do it for .wasp.mts. I know it is not likely that there will be .wasp.mts directory, but this way we are sure + we are consistent.

So what seems best to me now is to take this piece of logic that checks length and also add it to hasExtension, that because it really belongs there I think, and you also that way got it applied for both cases here + it is nicer to read.


hasExtension :: Path' s (File f) -> String -> Bool
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I don't like about this function is that it isnt' obvious if it takes .ext or ext as second argument, but it will not always work correctly in the latter case. Actually, if I had to guess, I would think it takes ext -> since that is extension, not .ext. I would either make so it exxpects ext + document that in a short comment, or remove the function and just inline this logic where it is used.

Another thing: you can go with (Path s) r (File f) as it doesn't matter if its Win or Posix path.

hasExtension path ext = ext `isSuffixOf` toFilePath path

analyzePackageJsonContent :: Path' Abs (Dir WaspProjectDir) -> IO (Either [CompileError] PackageJson)
analyzePackageJsonContent waspProjectDir =
findPackageJsonFile waspProjectDir >>= \case
Expand Down
6 changes: 2 additions & 4 deletions waspc/test/AnalyzerTest.hs
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ spec_Analyzer = do
)
( Just $
Job.ExecutorOptions
{ Job.pgBoss = JSON.JSON <$> Aeson.decode "{\"retryLimit\":1}",
Job.simple = Nothing
{ Job.pgBoss = JSON.JSON <$> Aeson.decode "{\"retryLimit\":1}"
}
)
let jobSchedule =
Expand All @@ -279,8 +278,7 @@ spec_Analyzer = do
(JSON.JSON <$> Aeson.decode "{\"job\":\"args\"}")
( Just $
Job.ExecutorOptions
{ Job.pgBoss = JSON.JSON <$> Aeson.decode "{\"retryLimit\":0}",
Job.simple = Nothing
{ Job.pgBoss = JSON.JSON <$> Aeson.decode "{\"retryLimit\":0}"
}
)
let expectedJob =
Expand Down
Loading