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.

Sign up for GitHub

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

Jump to bottom

Fix isInLink tracking and improve recording of transitive deps file: in locks #40

Closed
wants to merge 4 commits into from
Closed

Fix isInLink tracking and improve recording of transitive deps file: in locks #40

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b610af4
inflate-shrinkwrap: Insure modules deep in a linked tree are isInLink
iarna Aug 2, 2018
e694296
shrinkwrap: Don't iterate into linked deps
iarna Aug 2, 2018
03dade9
shrinkwrap: Don't record linked deps as bundled
iarna Aug 2, 2018
239a4aa
Update shrinkwrap.js
iarna Sep 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/install/inflate-shrinkwrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ function makeFakeChild (name, topPath, tree, sw, requested) {
realpath: requested.type === 'directory' ? requested.fetchSpec : childPath(tree.realpath, pkg),
location: (tree.location === '/' ? '' : tree.location + '/') + pkg.name,
isLink: requested.type === 'directory',
isInLink: tree.isLink,
isInLink: tree.isLink || tree.isInLink,
swRequires: sw.requires
})
tree.children.push(child)
Expand Down
7 changes: 5 additions & 2 deletions lib/shrinkwrap.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,13 @@ function shrinkwrapDeps (deps, top, tree, seen) {
var childIsOnlyDev = isOnlyDev(child)
var pkginfo = deps[moduleName(child)] = {}
var requested = getRequested(child) || child.package._requested || {}
var linked = child.isLink || child.isInLInk
Copy link

Choose a reason for hiding this comment

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

var linked = child.isLink || child.isInLInk

child.isInLink?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

child.isLink = "This module is a symlink in your node_modules

child.isInLink = "Some parent module of this module is a symlink"

Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be a typo with a capital i in Link in isInLink.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch, thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it was @christoffer-dropbox who caught it, I just wanted to clarify what I thought he meant.

var linkedFromHere = linked && path.relative(top.realpath, child.realpath)[0] !== '.'
pkginfo.version = childVersion(top, child, requested)
if (requested.type === 'git' && child.package._from) {
pkginfo.from = child.package._from
}
if (child.fromBundle || child.isInLink) {
if (child.fromBundle && !linked) {
pkginfo.bundled = true
} else {
if (isRegistry(requested)) {
Expand All @@ -139,7 +141,8 @@ function shrinkwrapDeps (deps, top, tree, seen) {
pkginfo.requires[moduleName(required)] = childRequested(top, required, requested)
})
}
if (child.children.length) {
// iterate into children on non-links and links contained within the top level package
if (child.children.length && (!child.isLink || linkedFromHere)) {
pkginfo.dependencies = {}
shrinkwrapDeps(pkginfo.dependencies, top, child, seen)
}
Expand Down
3 changes: 1 addition & 2 deletions test/tap/shrinkwrap-local-dependency.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ var shrinkwrap = {
version: 'file:' + unixFormatPath(path.join('mods', 'mod2')),
dependencies: {
mod1: {
version: 'file:' + unixFormatPath(path.join('mods', 'mod1')),
bundled: true
version: 'file:' + unixFormatPath(path.join('mods', 'mod1'))
}
}
}
Expand Down
4 changes: 1 addition & 3 deletions test/tap/spec-local-specifiers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,9 +585,7 @@ test('save behavior', function (t) {
var deps = pjson.dependencies || {}
t.is(deps['sb-transitive'], 'file:../sb-transitive', 'package.json')
var sdep = shrinkwrap.dependencies['sb-transitive'] || {}
var tdep = sdep.dependencies.sbta
t.like(tdep, {bundled: true, version: 'file:../sb-transitive/sbta'}, 'npm-shrinkwrap.json transitive dep')
t.like(sdep, {bundled: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep')
t.like(sdep, {bundled: null, dependencies: null, version: 'file:../sb-transitive'}, 'npm-shrinkwrap.json direct dep')
t.done()
})
})
Expand Down