duojs / duo

A next-generation package manager for the front-end
3.42k stars 118 forks source link

fixed endless while loop in find-root function #493

Closed bodokaiser closed 8 years ago

bodokaiser commented 8 years ago

Hey there again,

I have a (at the moment) private project which uses roo, duo and duo-babel so I can not 100% tell why I experience this issue and others not.

However what happens is that the while loop runs infinite as it converges to / I do not know why it does this (here are the var values):

To get a fix as quick as possible I just added a condition to the loop so it stops.

If somebody is interested in further investigation I can give them access to the repo for reproduction.

matthewmueller commented 8 years ago

dang, I thought this was fixed awhile ago, I wonder why it's converging to / and not .

dominicbarnes commented 8 years ago

It looks like something is wrong with the token that Travis CI is using, so the tests aren't passing. I'll look into this and fix it.

@bodokaiser I'm not really sure what the problem is here, as @matthewmueller mentioned this should've been fixed a while ago. Would you also be able to add a failing test that demonstrates the problem?

bodokaiser commented 8 years ago

Okay I got more details on what is going on. I was able to reproduce a small setup:

app/index.js

require('./foo');

app/foo/index.js

module.exports = 'bla'

lib/index.js

var duo  = require('duo');
var path = require('path');

duo(__dirname + '/..')
  .entry(__dirname + '/../app/index.js')
  .run(function() {
    console.info('success');
  });

If you run this with iojs lib then you will experience the described issue.

However it will not show up if you:

What seems to happen in Duo.prototype.findRoot which causes this behavior is that this.root returns the string Users/bodokaiser/src/github.com/bodokaiser/duo-test/lib/... Now the while loop calls dirname() on the given path /Users/bodokaiser/src/github.com/bodokaiser/duo-test/app/foo/index.js until it equals the root string. This however never happens because of lib/...

So one alternative solution I suggest would be to update this.root to call require('path').resolve(root) on the given root string which would remove /.. in the path.

dominicbarnes commented 8 years ago

Ok, there are a couple issues here:

1) you shouldn't use a path that contains .., you should really use path.resolve(__dirname, '..') 2) your entry file should be relative to the root you set earlier (ie: entry('app/index.js')

Try changing these things first to see if that fixes this problem.

bodokaiser commented 8 years ago

Hi, yes #1 using path.resolve(__dirname, '..') solved the problem. Thank you for taking a look into this!

Best Bodo