Unable to use modules from package set
JordanMartinez opened this issue · 9 comments
While #220 made the server work, the larger goal was to make this entire project work in a local setting. So, this issue seeks to fix that problem.
The current issue, explained by Thomas here, is that modules aren't known when we attempt to compile code that uses them.
Here's something I found. Notice how the externs and initNamesEnv argument are never even used in the server function?
I'm assuming that's the source of the issue here?
That’s definitely curious. I suspect it would be worth looking back through the history because I’m fairly certain they were used at one point.
I'm pretty sure this would have happened in #209.
Yes, looks like it. Specifically: 0427198
Edit: client, not trypurescript, server.
When I copy-paste the code we had originally in the commit linked above, the console of the client server produces this error message:
[2021-05-28T01:22:19.178Z] "GET /js/output/Data.Foldable/index.js" Error (404): "Not found"
[2021-05-28T01:22:19.179Z] "GET /js/output/Data.Functor/index.js" Error (404): "Not found"
[2021-05-28T01:22:19.179Z] "GET /js/output/TryPureScript/index.js" Error (404): "Not found"
So, copy-pasting the original code in the commit linked above and doing one other step (as a temporary fix below) seems to fix the issue. The above requests are being made because the returned JavaScript has require statements in it:
"use strict";
var Data_Foldable = require("../Data.Foldable/index.js");which will then ping the client server rather than the trypurescript server because they are on two different ports. If I run spago build in staging and copy staging/output folder to client/public/js/output, the local environment works.
So, put differently, why was the code transition to the buildMakeActions approach?
Ah right, I remember now - we transitioned to use P.rebuildModule because the previous code was pretty much an exact duplicate of that function's body, but the implementation of P.rebuildModule is likely to change in ways that have to be replicated here between compiler releases. Leaving it the way it was with the code duplicated would mean that we would have to take a lot more care when updating Try PureScript to a new compiler release.
This isn't just the development environment by the way, this problem would also surface in production. This is a must-fix before our next deploy, I think.
The current implementation seems to assume that we'll use rebuildModule', but we only need to use rebuildModule. See this line and the args it passes to rebuildModule'.
So, I think we need to make the following changes. This doesn't completely fix the problem because the require statements above are asking the client server (i.e. client/public) rather than the compile server (e.g. server), but this at least fixes the module problem I believe.
e <- runExceptT $ do
modules <- ExceptT $ I.loadAllModules inputFiles
- (exts, env) <- ExceptT . I.runMake . I.make $ map (second CST.pureResult) modules
- namesEnv <- fmap fst . runWriterT $ foldM P.externsEnv P.primEnv exts
+ ExceptT . I.runMake . I.make $ map (second CST.pureResult) modules
- pure (exts, namesEnv, env)
case e of
Left err -> print err >> exitFailure
- Right (exts, namesEnv, env) -> server exts namesEnv env port
+ Right (exts, env) -> server exts env port- server :: [P.ExternsFile] -> P.Env -> P.Environment -> Int -> IO ()
- server externs initNamesEnv initEnv port = do
+ server :: [P.ExternsFile] -> P.Environment -> Int -> IO ()
+ server externs initEnv port = do- (makeResult, warnings) <- Make.runMake P.defaultOptions $ Make.rebuildModule makeActions [] m
+ (makeResult, warnings) <- Make.runMake P.defaultOptions $ Make.rebuildModule makeActions externs m