Fibers compilation thru require() hook causes repeated errors

[aseemk]

I haven't figured out how to make a simple repro for this, but figured I'd open this issue with the info I have so far.

We've always compiled and run our app in callbacks mode. I've recently started experimenting with fibers mode for improved debuggability. Initially, it didn't compile, but it failed with the same file each time, so I was able to debug it and find the root cause (issue #185).

Now, with Streamline 0.10.3, all of our files compile perfectly fine when compiled explicitly, via _coffee --fibers --compile <...>. (The resulting code doesn't run error-free yet, but we'll get there later.) However, if I try to run it via a require() hook — require('streamline').register({fibers: true}) — then it doesn't compile yet.

But the crazy thing is, the file it fails on is different every time. If our entry point is file A, and it requires file B, and that requires file C, and so on, the first time the error message is for file A, then if I try again it's for file B, and so on. We have probably over a hundred files, so I haven't finished retrying 100+ times to see if works by the end yet. I'll write a loop to do that soon.

The error message is always the same, TypeError: Object <the whole compiled file> has no method 'replace'. E.g.:

TypeError: Object var fstreamline__ = require("streamline/lib/fibers/runtime"); (fstreamline__.create(function(_) {
// our fibers-compiled JS code...
// e.g.
this.meta.get = fstreamline__.create(function(req, res, _) {
// ...
})); has no method 'replace'
    at Module._compile (module.js:377:21)
    at Object.requireSync (/path/to/our/app/node_modules/coffee-streamline/coffee-streamline.js:214:12)
    at Object.._coffee (/path/to/our/app/node_modules/node-dev/lib/hook.js:52:17)
    at Module.load (/path/to/our/app/node_modules/coffee-script/lib/coffee-script/coffee-script.js:211:36)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at requireDir (/path/to/our/app/node_modules/require-dir/index.js:112:33)
    at Object.<anonymous> (/path/to/our/app/api/app._coffee:17:8)
    at __streamline$run (eval at makeCreator (/path/to/our/app/node_modules/streamline/lib/fibers/runtime.js:76:89), <anonymous>:16:14)

Any idea what might be happening?

[aseemk]

We have probably over a hundred files, so I haven't finished retrying 100+ times to see if works by the end yet. I'll write a loop to do that soon.

Looped, and yes, it finally does run by the end. But then we're back to the same runtime errors we see when we explicitly pre-compile and run with staple node. I'll debug those on my own separately.

[bjouhier]

It is because the signature of the transform method has changed. When sourcemap is active, transform returns a SourceNode rather than a string. This breaks coffee-streamline's requireSync which expects it to return a string.

I can fix it by renaming transform inside streamline and reintroducing a transform that returns a string, like before.

[aseemk]

Ah, thanks for catching that! No need to change anything on your end. I'll update coffee-streamline.

Btw, coffee-streamline was a quick hack, always meant as a temporary solution to the problem of slow compilation times. You added caching to the Streamline compilation part, but it still doesn't cache the CoffeeScript compilation in the case of ._coffee files. I did a quick test again on our app recently, and that does add several seconds of difference. Totally cool if you think caching CoffeeScript compilation is out of scope for Streamline's compilation caching, but it'd be awesome if that could be baked in, too. Thoughts?

[bjouhier]

Why do you want to cache the intermediate CS file? If the *._coffee source changes you need to run both passes (CS and streamline) anyway? But I may be missing something (debuggability?)

I still think that I should fix transform because it is odd to have a public API that returns different result types depending on an option.

[aseemk]

Ah, I wasn't clear. What I meant was: Streamline's require() hook for ._coffee files always passes the file to CoffeeScript's compiler, even with the --cache option. (That option just skips Streamline's JS compiler.)

https://github.com/Sage/streamlinejs/blob/master/lib/compiler/underscored.js#L123-L142

coffee-streamline, on the other hand, skips both compilers if the source file hasn't changed.

https://github.com/aseemk/coffee-streamline/blob/master/coffee-streamline.js#L199-L211

Good call on fixing the API either way. Should I hold off on making any changes to coffee-streamline then?

[aseemk]

Further clarification: Streamline's --cache option is only used in the transform layer right now. coffee-streamline caches at the require() hook layer.

[bjouhier]

OK. I get it now. This should be fixed in the streamline layer.

I'm going to fix the transform call but not tonite. So you may need a temporary fix.
Will you still need coffee-streamline if the streamline cache avoids the CS compilation pass?

[aseemk]

Awesome, thanks Bruno.

I wouldn't like to continue using coffee-streamline — I see it as a hack — so if Streamline avoided CS re-compilation, and assuming a quick comparison then made Streamline just as fast as coffee-streamline, I would indeed no longer need coffee-streamline. =)

[bjouhier]

@aseemk I enabled the caching improvement to avoid the coffeescript compilation pass when the result is already cached.

After more thought (and a dose of laziness) I'm not going to touch the transform API. I just need to document that it returns an object instead of a string when the sourceMap option is set. So people who use this API should not set the sourceMap option if they want a string in return. No need to make things more complicated with a dual API (an internal one that would return the complex result and an external one that would always return a string).

[aseemk]

Sounds good. I'll update coffee-streamline. Thanks Bruno!