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

robot.load tries to load both .coffee and .js versions of script when transpiled #653

Closed
ppatarinski opened this issue Feb 17, 2014 · 8 comments

Comments

@ppatarinski
Copy link

Hi all,

There is an issue with the Load function

Fs.exists path, (exists) =>
if exists
for file in Fs.readdirSync(path).sort()
@loadfile path, file

The issue is present when you have both a js file and a coffee script file in the scripts folder (this happens when you transpile the coffee script (produces a js file in the same directory)). Since the for loop is not checking if a script by that name is already added it will add both the coffee script and the javascript, resulting in a duplicate.
So when you call
hubot ping
you get
pong
pong

I am very new to coffee script, so not going to try to submit a patch, but here is my 2 cents on fixing it.

When you are iterating through the files and the ext is '.js' replace the ext with '.coffee' and check if a file by that name is already loaded. If so do not load it.

Cheers
Paul

@technicalpickles
Copy link
Member

That makes sense I think.

I'm curious, how are you getting compiled javascript files in scripts directory? hubot is normally run in-line, so it wouldn't compile by default, instead just compiling in memory and executing them that way.

@ppatarinski
Copy link
Author

Here are the steps to get the compiled js.

  1. Open hubot in either IntelliJ or Visual Studio (https://github.com/ppatarinski/Hubot_VisualStudio)
  2. In order to debug coffee script, you will need to transpile it to javascript
  3. Get a transpiler for coffee script added to the IDE (in Visual Studio that is Web Essentials)
  4. Transpile all the coffee script files in the scripts directory (produces .js files for each one)
  5. Make hubot.js your starting file
  6. Put a breakpoint in one the js script files
  7. Run the project, which starts hubot in command line
  8. Call a hubot command and watch the js script hit.

@technicalpickles
Copy link
Member

I wrote this in email, but posting here for completeness:

I'm curious, is there a way to have it transpile coffeescript files into javascript inside of a different directory? Like, I used to do Java development in Eclipse, and you could have it compile .java into .class in a separate classes/ directory. One problem I see with transpiling .js in the scripts directory is that it makes it hard to have js scripts, instead of coffee scripts while at the same point preventing transpiled js to not be checked into version control. Having it in a separate directory makes it easy to gitignore that directory.

@technicalpickles
Copy link
Member

Updated title to reflect the issue.

@mdarveau
Copy link

It's wierd that the script get's loaded twice since loadFile loads the .js only if there is not .coffee with the same filename:

if ext is '.coffee' or ( ext is '.js' and not Fs.existsSync( Path.join( path, Path.basename( file, ext ) + '.coffee' ) ) )

I also have issues debugging my bot because of this. I was about to send a pull request so loadHubotScripts ignore .coffee if there is a .js file with the same name. This would assume that the .js is transpiled and up to date with the .coffee counterpart.

To answer technicalpickles on why this is useful, here how it works in IntelliJ. IntelliJ does not support debugging of coffeescript but can debug node/javascript. However, it does support map files generated by the coffeescript transpiler to allow breakpoints and some debugging (see current execution line, step, basic variable inspection, ...) from the .coffee file. But all this is just mapping magic, not real coffeescript debugging.
Basically, to debug a coffeescript program, you transpile it to javascript and then run/debug on the javascript file. If that file tries to load a .coffee file (require a .coffee file), it fails as it's a pure javascript executor. Typically, you do not require somefile.coffee nor require somefile.js but simply require somefile. The node/coffee script runtime will then add the .coffee or .js suffix.
The file needs to be in the same directory as the coffee file for the require to work. This is why you can't just put the .js files elsewhere. I am not a node expert but I don't think you can add additional directories to the "classpath". And I don't see how this would work when files are referenced by path like require '../../commons/utils'

In the case of the robot loadFile, it takes a file name with the .coffee or .js extension, does the check I copied above and then require somefile. Maybe we could make the extension optional for scripts in hubot-scripts.json, and modify loadFile to just require the file? We would then have to modify load to skip .coffee if there is a .js beside.

Does it makes sense?

@mdarveau
Copy link

Any comment on this? I am willing to send a pull request but I don't want to send something that's too far off.

@technicalpickles
Copy link
Member

I think this makes sense, so 👍 to a PR

cc #667 since it's sorta in the same spot in the code base

@michaelansel
Copy link
Collaborator

c.f. #648 since this is probably useful information to include in the debugging docs

@stale stale bot added the stale label May 30, 2017
@stale stale bot closed this as completed Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants