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

Unable to use modules from package set #222

Closed
JordanMartinez opened this issue May 27, 2021 · 9 comments · Fixed by #224
Closed

Unable to use modules from package set #222

JordanMartinez opened this issue May 27, 2021 · 9 comments · Fixed by #224
Assignees

Comments

@JordanMartinez
Copy link
Contributor

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.

@JordanMartinez
Copy link
Contributor Author

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?

@hdgarrood
Copy link
Collaborator

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.

@thomashoneyman
Copy link
Member

I'm pretty sure this would have happened in #209.

@thomashoneyman
Copy link
Member

thomashoneyman commented May 27, 2021

Yes, looks like it. Specifically: 0427198

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented May 28, 2021

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"

@JordanMartinez
Copy link
Contributor Author

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?

@hdgarrood
Copy link
Collaborator

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.

@hdgarrood
Copy link
Collaborator

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.

@hdgarrood hdgarrood changed the title Get development environment to work again Unable to use modules from package set May 28, 2021
@thomashoneyman thomashoneyman self-assigned this May 28, 2021
@JordanMartinez
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants