ybogdanov / node-sync

Write simple and readable synchronous code in nodejs using fibers
MIT License
492 stars 70 forks source link

Auto create Fiber when calling sync() on a function #22

Closed fzaninotto closed 11 years ago

fzaninotto commented 11 years ago

At the moment, if not run inside a Sync() with a callback, calling sync() on a Function throws the following error:

/path/to/node_modules/sync/lib/sync.js:58
        if (yielded) fiber.run();
                           ^
TypeError: Cannot call method 'run' of undefined
    at Function.sync.syncCallback (/path/to/node_modules/sync/lib/sync.js:58:28)
    at Object.oncomplete (fs.js:297:15)

It would be very convenient to allow each Function to run in its own Fiber, i.e. create one if it doesn't exist yet, so that on could call:

fs.readdir.sync(null, './');

instead of:

Sync(function() {
  fs.readdir.sync(null, './');
});

Is there a limitation in the Fiber lib that prevents it from working?

ybogdanov commented 11 years ago

Yep, there's a limitation. Fiber should wrap the code you want to be "synced". It should be outside. Running fiber inside of Function.prototype.sync will not work.

fzaninotto commented 11 years ago

OK. From reading the Fiber documentation, I thought this was possible. Here is one example they give:

var Fiber = require('fibers');

// Generator function. Returns a function which returns incrementing
// Fibonacci numbers with each call.
function Fibonacci() {
    // Create a new fiber which yields sequential Fibonacci numbers
    var fiber = Fiber(function() {
        Fiber.yield(0); // F(0) -> 0
        var prev = 0, curr = 1;
        while (true) {
            Fiber.yield(curr);
            var tmp = prev + curr;
            prev = curr;
            curr = tmp;
        }
    });
    // Return a bound handle to `run` on this fiber
    return fiber.run.bind(fiber);
}

// Initialize a new Fibonacci sequence and iterate up to 1597
var seq = Fibonacci();
for (var ii = seq(); ii <= 1597; ii = seq()) {
    console.log(ii);
}

So you say you can't create a Fiber on the fly and run it as in this example?

ybogdanov commented 11 years ago

Really. Now it works, seems that something was changed to fibers & node recently. I will think about how I can adopt node-sync to this.

Thanks for your request!

fzaninotto commented 11 years ago

If you can implement it, it will change the life of generations of Node.js developers stuck in callback spaghetti... Looking forward to using your module !

ybogdanov commented 11 years ago

Ah, got it. It can't be asynchronous. Fibonacci example is fully synchronous. If you'll try to add setTimeout or process.nextTick there - it will fail.

With express you can wrap each request with fiber automatically by using Function.prototype.asyncMiddleware mixin. I've added an example for you: https://github.com/0ctave/node-sync/blob/1d567bac8ae5bc9982372ab89ab9a1b1021ab923/examples/express.js

fzaninotto commented 11 years ago

Ok, thanks for taking a look at this anyway.