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

Doesn't fail if a dependency is not loaded #1

Closed
bengourley opened this issue Aug 12, 2014 · 7 comments
Closed

Doesn't fail if a dependency is not loaded #1

bengourley opened this issue Aug 12, 2014 · 7 comments

Comments

@bengourley
Copy link

Example below. I would expect an error to be passed as the first argument where I am doing a console.log(). Not only does it not get an error, but it looks like the callback is not called at all and this program just terminates.

var loader = require('component-loader')

loader([ component ], function (cb) { return cb }, function () {
  console.log(arguments)
})

function component() {
  return { component: [ 'nonExistentCompent', init ] }
  function init(cb) { cb() }
}
@bengourley
Copy link
Author

This is actually really posing a problem. If you specify a dependency and it doesn't exist, your callback is never called and you don't know what one is missing. If you have lots of components it makes it really hard to track down.

@microadam
Copy link
Owner

Hmm, it just uses async.auto under the hood, so its quite possible that its a problem with that

@bengourley
Copy link
Author

The async maintainer closed a feature request for it: caolan/async#263

This is a pretty fundamental issue with component-loader and definitely needs fixing. If we are going to go the dependency-resolution route it needs to work properly.

@microadam
Copy link
Owner

Thought I may have been stung by this problem, so I wrote the following to test:

  var _ = require('lodash')
    , componentNames = []
    , dependencies = []

  components.forEach(function (component) {
    var c = component()
      , name = Object.keys(c)[0]
      , deps = c[name]

    componentNames.push(name)
    if (Array.isArray(deps)) {
      deps.pop()
      dependencies = dependencies.concat(deps)
    }
  })

  dependencies = _.uniq(dependencies)
  var missingDependencies = _.difference(dependencies, componentNames)
  console.log(missingDependencies)

Just putting here so it doesn't get lost. Will integrate into the module and release a new version when I can

@domharrington
Copy link

Circular dependencies also cause a problem. This may be a bug with the code but it becomes very hard to track this down when async.auto never calls the callback.

var loader = require('component-loader')

loader([ a, b ], function (cb) { return cb }, function () {
  console.log(arguments)
})

function a() {
  return { a: [ 'b', init ] }
  function init(cb) { cb() }
}

function b() {
  return { b: [ 'a', init ] }
  function init(cb) { cb() }
}

@serby
Copy link
Contributor

serby commented Dec 23, 2014

Olly just fell foul of this issue too. There must be something can can be done in component-loader to make this throw a decent error if there is a cyclic dependancy graph or a missing dependancy.

@microadam
Copy link
Owner

Both situations now throw an error in v0.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants