duojs / duo

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

Duo not importing a repo with a component.json #499

Closed ismay closed 9 years ago

ismay commented 9 years ago

A component.json file was recently added to PhotoSwipe. As far as I can tell it conforms to the component.json spec, and PhotoSwipe has a commonjs export defined.

Photoswipe doesn't have an index.js entry point, but the dist/photoswipe.js entry point is defined in the main key of it's component.json. The weird thing is that when I require photoswipe, it gets downloaded to my components folder, but it isn't included in duo.json.

So after the build PhotoSwipe is undefined in index.js. Am I doing something wrong here, or is Duo not importing PhotoSwipe correctly?

Branch with demonstration of the problem: https://github.com/ismay/ismaywolff.nl/commit/67831fb6ffe45ee6982aa0884ce9b81c08c86e3e

dominicbarnes commented 9 years ago

Aha, I figured out what is going on.

They've added the component.json to their master branch, but it's not on the latest tagged version. (ie: v4.1.0)

If you switch to dimsemenov/PhotoSwipe@master in the interim, it will work. (once they tag a new version, you can go back to using semver)

ismay commented 9 years ago

Ah of course, that makes sense. Thanks for the help!

ismay commented 9 years ago

So just so I understand, duo doesn't fall back to master if there isn't a tagged version with a component.json? I can see how such a fallback would be problematic since you can't always count on there being a master branch, but maybe in that case an error message would help with debugging this kind of issue?

But I don't know whether that fits with Duo's design philosophy. Just a thought.

dominicbarnes commented 9 years ago

Duo will fall back to master if there are no tags. However, it doesn't assume that a missing component.json is an error if there are already tags available.

As of now, it is kinda complex internally to unwind once the resolution has been completed. Falling back to master is kind of a hack anyways, which should only happen when there are no tags.

ismay commented 9 years ago

Ah ok, so in this case it's a combination of a missing component.json in the latest tagged version, and a non-standard entry point (i.e. no index.js).

dominicbarnes commented 9 years ago

Yeah, this is a one-time issue for this module. For any versions moving forward, this won't be a problem.