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

pulp run's loader stub file can interfere with module names, should be kept in a temp directory #26

Closed
felixSchl opened this issue Mar 25, 2015 · 9 comments

Comments

@felixSchl
Copy link

The README does not make specific mention of it but am I correct to assume that a pulp init && pulp run should work without errors?

I am getting:

...
on (exports, require, module, __filename, __dirname) { require('Main').main();
                                                                       ^
TypeError: Object #<Object> has no method 'main'
...
@felixSchl
Copy link
Author

pulp init && pulp test works, btw. I think it has to do with the node env not being set correctly.

@felixSchl
Copy link
Author

It's because main.js is picked up as the main module, but the default skeleton creates a Main purescript module, which results in output/Main/index.js. Node will import main.js on require('Main'), rather than Main/index.js.

felixSchl added a commit to felixSchl/pulp that referenced this issue Mar 25, 2015
`main.js` clashes with the skeleton's default `Main` module - Node
will load `main.js`, rather than `Main/index.js` upon calling
`require('Main')`. By renaming from `main.js` to something else,
node will load the proper module.

Fixes purescript-contrib#26
@bodil
Copy link
Collaborator

bodil commented Mar 25, 2015

Proposed solution would be to not store the wrapper file inside the build directory at all, rather using the temp module to store it somewhere out of the way, then get rid of it automatically when it's no longer needed.

@bodil bodil changed the title Pulp init >> pulp run pulp run's loader stub file can interfere with module names, should be kept in a temp directory Mar 25, 2015
@felixSchl
Copy link
Author

I appreciate it's not an all encompassing solution but that is besides the point of the pull request. The initial experience of starting with pulp is what mattered to me. If I change things, I can expect unforeseen things to happen.

@bodil
Copy link
Collaborator

bodil commented Mar 25, 2015

I appreciate that, I'd still like to get the correct solution in asap. And I'm working on it as we speak, give me a moment. :)

@bodil bodil closed this as completed in c7709b7 Mar 25, 2015
@felixSchl
Copy link
Author

That commit introduces a Type error in run.js on line 22. Just add the additional parameter to fs.write and it works: felixSchl/pulp@908600e

@bodil
Copy link
Collaborator

bodil commented Mar 26, 2015

That's curious. What Node version are you on, just for reference? The docs have that parameter down as optional.

@felixSchl
Copy link
Author

The docs specify the parameter as optional only if the other optional parameter position has been specified - note the nested brackets. You can refer to the implementation which has not changed since mid july 2013.

@bodil
Copy link
Collaborator

bodil commented Mar 26, 2015

The code seems to willingly accept both, but I agree with your interpretation of the doc. JS is terrible. :(

It did run correctly in its previous form on my v0.12.0 version of Node, though, which is why I'm puzzled.

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

Successfully merging a pull request may close this issue.

2 participants