maxtaco / coffee-script

IcedCoffeeScript
http://maxtaco.github.com/coffee-script
MIT License
727 stars 58 forks source link

Bug introduced in 1.7.1-c: Cannot find module 'iced-runtime' #121

Closed icflorescu closed 10 years ago

icflorescu commented 10 years ago

Hi Max,

This must be somehow related to moving the iced-runtime in a separate package. After upgrading to 1.7.1-c I'm now getting this kind of error in most of my projects. Unfortunately I can't really pinpoint what's happening, but here's a test case:

app.iced:

app          = require('express')()
server       = require('http').createServer app
io           = require('socket.io').listen server
logger       = require 'morgan'
session      = require 'express-session'
compression  = require 'compression'
bodyParser   = require 'body-parser'
cookieParser = require 'cookie-parser'
SessionStore = require './utils/session-store'
mongoose     = require 'mongoose'
st           = require 'st'

...

app.use compression()
app.use bodyParser()
app.use cookieParser()

...

require('./controllers/authentication') app
require('./controllers/pages') app
require('./controllers/api') app, io

# Start listening
server.listen env.PORT, -> console.log "Application instance listening on port #{env.PORT}..."

./controllers/api.iced:

Deal = require '../models/deal'

module.exports = (app, io) ->

  app.all '/api/*', (req, res, next) ->
    return res.send 401 unless req.isAuthenticated()
    next()

...

I'm getting this error while trying to run the application with 1.7.1-c:

Error: Cannot find module 'iced-runtime'
  at Function.Module._resolveFilename (module.js:338:15)
  at Function.Module._load (module.js:280:25)
  at Module.require (module.js:364:17)
  at require (module.js:380:17)
  at Object.<anonymous> (/home/ionut/Work/lsh-sales/web/controllers/api.iced:1:1)
  at Object.<anonymous> (/home/ionut/Work/lsh-sales/web/controllers/api.iced:1:1)
  at Module._compile (module.js:456:26)
  at Object.loadFile (/usr/lib/node_modules/iced-coffee-script/lib/coffee-script/register.js:16:19)
  at Module.load (/usr/lib/node_modules/iced-coffee-script/lib/coffee-script/register.js:45:36)
  at Function.Module._load (module.js:312:12)
  at Module.require (module.js:364:17)
  at require (module.js:380:17)
  at Object.<anonymous> (/home/ionut/Work/lsh-sales/web/app.iced:59:1)
  at Object.<anonymous> (/home/ionut/Work/lsh-sales/web/app.iced:1:1)
  at Module._compile (module.js:456:26)

Everything used to work fine up until 1.7.1-b.

I can't really figure out why it breaks when requireing api.iced - it's not even the first require...

I'm also having similar issues in ASPAX (a simple asset packager written entirely in ICS) and some of its plugins. So, for now I had to freeze ICS on ASPAX to 1.7.1-b...

Any idea?

maxtaco commented 10 years ago

Does it fix the issue to npm install -d --save iced-runtime ?

icflorescu commented 10 years ago

Yes, apparently it does. So does installing 1.7.1-d directly from github. :-)

Thanks a lot for your quick reply!

P.S. Other people may run into this, so it may be a good idea to leave the issue open until you publish 1.7.1-d to npm.

maxtaco commented 10 years ago

I see, that's not a great fix though, to have to install iced-runtime globally in addition to iced-coffee-script. Let me play around with a few other options...

icflorescu commented 10 years ago

Well, apparently if you're using 1.7.1-d from github locally in a project, you don't have to install iced-runtime:

package.json:

"dependencies": {
  "iced-coffee-script": "maxtaco/coffee-script",
  ...,
  "underscore": "^1.6.0"
}

I haven't tested it globally yet, but I'll give it a try and let you know. Maybe it's something you've fixed in this commit?

maxtaco commented 10 years ago

I think it's fixed in f4ec5f9. When running iced foo.iced, it should look for the runtime internal to the compiler/interpreter. I think this is good for most/all situations. Otherwise, it will look in the independent runtime.

The advantage of all of this, BTW, is that now you can release "iced" packages without including the compiler, which has gotten bloated with the recent docco/highlighting changes.

icflorescu commented 10 years ago

With f4ec5f9 (and the latest 1.7.1-d published on npm), if you're using iced-coffee-script locally in your project, you do have to include iced-runtime in package.json:

"dependencies": {
  "iced-coffee-script": "^1.7.1-d",
  "iced-runtime": "^0.0.1",
  ...
}

I'm ok with it I guess, but people will bump into it sooner or later and you'll probably get quite a number of github issues... Maybe it would be a good idea to make a note somewhere in the README.md.

maxtaco commented 10 years ago

Let me take a look -- which project is it? ASPAX? And what steps should I take to reproduce the issue?

maxtaco commented 10 years ago

Aha, I see, maybe register isn't working properly? On it...

icflorescu commented 10 years ago

Yes, ASPAX has the same issue, but it may be difficult to test. I'll try setting up another test case and get back to you...

maxtaco commented 10 years ago

Ok, maybe it's better now? For running iced foo.iced and for running require 'iced-coffee-script/register, it's using the runtime that comes with iced-coffee-script.

icflorescu commented 10 years ago

Yep! 1.7.1-e works perfectly, no need to add iced-runtime to dependencies now! Let me do a quick test to see how this works in ASPAX...

icflorescu commented 10 years ago

Everything's just fine.

Thanks again for your quick reply!

maxtaco commented 10 years ago

Excellent, glad to hear it.

I have a lot of projects in which I compile and then ship the compiled .js code to npm. For these, the idea is to have iced-coffee-script as a devDependency and iced-runtime as a proper dependencies. This way, only the latter is fetched on npm install.

icflorescu commented 10 years ago

Interesting idea.

I was using a manually generated runtime in compiled client-side scripts (packed with ASPAX).

Personally I like CoffeeScript very much, I'm also a big fan of ICS and I try to do my best to "promote" it whenever I can (like here, here and here :-). I don't mind using them as proper dependencies on the server-side. I know there are narrow-minded "religious" people out there criticizing them for not being "pure", but I prefer using the right tool for the right job. And ICS does make you feel like eating your cake and keeping it too :-)

But being able to deploy without an iced-coffee-script dependency does sound interesting!

I'll have a look at iced-runtime to figure out the workflow when I'll find some spare time. Writing/sharing a tutorial would be nice and it would bring more awareness to the project; i.e. I know people who were initially put off entirely by Node.js callback hell, but decided to switch on as soon as they saw ICS.

Thanks again for doing such a great job in the community! ;-)

vmakhaev commented 10 years ago

For browserify you should add iced-runtime by hand: browserify.add 'iced-runtime'

Otherwise you will have this error.

zapu commented 10 years ago

I have similar errors with blanketjs and mocha. For now, I'm just installing iced-runtime globally, but that's not a real solution...