Skip to content
This repository has been archived by the owner on Mar 4, 2022. It is now read-only.

Change to more permissive sibling dependency inference. #31

Merged
merged 1 commit into from
Nov 25, 2015

Conversation

ryan-roemer
Copy link
Member

Fixes #30

  • Infer sibiling dependencies as a fallback if installed from NPM at all (v3 or v2)
  • Also throw in a require.resolve as another fallback method.

/cc @exogen @boygirl

@ryan-roemer
Copy link
Member Author

I have a successful local install of victory-axis using this version for both npm v2 and npm v3.

@boygirl
Copy link
Contributor

boygirl commented Nov 25, 2015

awesome! can you please do a version bump with this fix?

@ryan-roemer
Copy link
Member Author

@exogen -- I'm going to merge to get @boygirl going and while I'm actually available. Would love a post-merge review from you!

ryan-roemer added a commit that referenced this pull request Nov 25, 2015
Change to more permissive sibling dependency inference.
@ryan-roemer ryan-roemer merged commit 01f53d1 into master Nov 25, 2015
@ryan-roemer ryan-roemer deleted the bug-npmv2-sibling-dedupe-deps branch November 25, 2015 23:38
// Require resolve it
var modPath = require.resolve(name);
pkgPath = path.join(modPath, "package.json");
return require(pkgPath);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break if the archetype has a main file (our current archetype doesn't, but I don't think we should break here if an archetype has one for some reason). For example when I try this on lodash:

> require("path").join(require.resolve("lodash"), "package.json")
'/Users/brianbeck/Projects/Tests/npm-test/node_modules/lodash/dist/lodash.js/package.json'

Also, is there a good reason not to just use require.resolve as the one-and-only way? I really believe that needing to crawl up the tree to find the archetype will end up being the common case, not an edge case.

@ryan-roemer
Copy link
Member Author

Released as builder@2.1.2.

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

Successfully merging this pull request may close these issues.

BUG: on npm 2 builder postinstall fails if builder package includes another builder package
3 participants