Skip to content

Commit

Permalink
perf: symlink leaf dependencies
Browse files Browse the repository at this point in the history
Close #789
  • Loading branch information
zkochan committed Jun 13, 2017
1 parent 89ca932 commit c69bea6
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 9 deletions.
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -41,6 +41,7 @@
"graceful-fs": "^4.1.11",
"is-ci": "^1.0.10",
"is-inner-link": "^2.0.0",
"is-subdir": "^1.0.0",
"is-windows": "^1.0.0",
"load-json-file": "^2.0.0",
"load-yaml-file": "^0.1.0",
Expand Down
2 changes: 2 additions & 0 deletions shrinkwrap.yaml
Expand Up @@ -25,6 +25,7 @@ dependencies:
graceful-fs: 4.1.11
is-ci: 1.0.10
is-inner-link: 2.0.0
is-subdir: 1.0.0
is-windows: 1.0.1
isexe: 2.0.0
load-json-file: 2.0.0
Expand Down Expand Up @@ -3526,6 +3527,7 @@ specifiers:
in-publish: ^2.0.0
is-ci: ^1.0.10
is-inner-link: ^2.0.0
is-subdir: ^1.0.0
is-windows: ^1.0.0
isexe: ^2.0.0
load-json-file: ^2.0.0
Expand Down
3 changes: 2 additions & 1 deletion src/api/install.ts
Expand Up @@ -255,7 +255,8 @@ async function installInContext (
parentNodeId: ':/:',
currentDepth: 0,
}
const nonLinkedPkgs = await pFilter(packagesToInstall, (spec: PackageSpec) => !spec.name || safeIsInnerLink(nodeModulesPath, spec.name))
const nonLinkedPkgs = await pFilter(packagesToInstall,
(spec: PackageSpec) => !spec.name || safeIsInnerLink(nodeModulesPath, spec.name, {storePath: ctx.storePath}))
const rootPkgs = await installMultiple(
installCtx,
nonLinkedPkgs,
Expand Down
12 changes: 9 additions & 3 deletions src/api/uninstall.ts
Expand Up @@ -62,7 +62,7 @@ export async function uninstallInContext (pkgsToUninstall: string[], pkg: Packag
storePath: ctx.storePath,
skipped: Array.from(ctx.skipped).filter(pkgId => removedPkgIds.indexOf(pkgId) === -1),
})
await removeOuterLinks(pkgsToUninstall, path.join(ctx.root, 'node_modules'))
await removeOuterLinks(pkgsToUninstall, path.join(ctx.root, 'node_modules'), {storePath: ctx.storePath})
}
}

Expand All @@ -75,9 +75,15 @@ function isDependentOn (pkg: Package, depName: string): boolean {
.some(deptype => pkg[deptype] && pkg[deptype][depName])
}

async function removeOuterLinks (pkgsToUninstall: string[], modules: string) {
async function removeOuterLinks (
pkgsToUninstall: string[],
modules: string,
opts: {
storePath: string,
}
) {
for (const pkgToUninstall of pkgsToUninstall) {
if (!await safeIsInnerLink(modules, pkgToUninstall)) {
if (!await safeIsInnerLink(modules, pkgToUninstall, opts)) {
await removeTopDependency(pkgToUninstall, modules)
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/link/index.ts
Expand Up @@ -202,7 +202,10 @@ async function linkPkg (
) {
const fetchResult = await dependency.fetchingFiles

if (dependency.independent) return

const pkgJsonPath = path.join(dependency.hardlinkedLocation, 'package.json')

if (fetchResult.isNew || opts.force || !await exists(pkgJsonPath) || !await pkgLinkedToStore(pkgJsonPath, dependency)) {
await linkIndexedDir(dependency.path, dependency.hardlinkedLocation, fetchResult.index)
}
Expand Down
12 changes: 10 additions & 2 deletions src/link/resolvePeers.ts
Expand Up @@ -16,6 +16,9 @@ export type DependencyTreeNode = {
resolution: Resolution,
hardlinkedLocation: string,
children: string[],
// an independent package is a package that
// has neither regular nor peer dependencies
independent: boolean,
optionalDependencies: Set<string>,
depth: number,
resolvedId: string,
Expand Down Expand Up @@ -94,15 +97,20 @@ function resolvePeersOfNode (

nodeIdToResolvedId[nodeId] = resolvedId
if (!resolvedTree[resolvedId] || resolvedTree[resolvedId].depth > node.depth) {
const hardlinkedLocation = path.join(modules, node.pkg.name)
const independent = !node.children.length && R.isEmpty(node.pkg.peerDependencies)
const pathToUnpacked = path.join(node.pkg.path, 'node_modules', node.pkg.name)
const hardlinkedLocation = !independent
? path.join(modules, node.pkg.name)
: pathToUnpacked
resolvedTree[resolvedId] = {
name: node.pkg.name,
hasBundledDependencies: node.pkg.hasBundledDependencies,
fetchingFiles: node.pkg.fetchingFiles,
resolution: node.pkg.resolution,
path: path.join(node.pkg.path, 'node_modules', node.pkg.name),
path: pathToUnpacked,
modules,
hardlinkedLocation,
independent,
optionalDependencies: node.pkg.optionalDependencies,
children: R.union(node.children, resolvedPeers),
depth: node.depth,
Expand Down
11 changes: 10 additions & 1 deletion src/safeIsInnerLink.ts
Expand Up @@ -3,13 +3,22 @@ import path = require('path')
import isInnerLink = require('is-inner-link')
import fs = require('mz/fs')
import mkdirp = require('mkdirp-promise')
import isSubdir = require('is-subdir')

export default async function safeIsInnerLink (modules: string, depName: string) {
export default async function safeIsInnerLink (
modules: string,
depName: string,
opts: {
storePath: string,
}
) {
try {
const link = await isInnerLink(modules, depName)

if (link.isInner) return true

if (isSubdir(opts.storePath, link.target)) return true

logger.info(`${depName} is linked to ${modules} from ${link.target}`)
return false
} catch (err) {
Expand Down
9 changes: 7 additions & 2 deletions test/shrinkwrap.ts
Expand Up @@ -271,11 +271,16 @@ test('package is not marked dev if it is also a subdep of a regular dependency',
await addDistTag('dep-of-pkg-with-1-dep', '100.0.0', 'latest')

await installPkgs(['pkg-with-1-dep'], testDefaults())

t.pass('installed pkg-with-1-dep')

await installPkgs(['dep-of-pkg-with-1-dep'], testDefaults({saveDev: true}))

t.pass('installed optional dependency which is also a dependency of pkg-with-1-dep')

const shr = await project.loadShrinkwrap()

t.notOk(shr.packages['/dep-of-pkg-with-1-dep/100.0.0']['dev'])
t.notOk(shr.packages['/dep-of-pkg-with-1-dep/100.0.0']['dev'], 'package is not marked as dev')
})

test('package is not marked optional if it is also a subdep of a regular dependency', async (t: tape.Test) => {
Expand All @@ -288,5 +293,5 @@ test('package is not marked optional if it is also a subdep of a regular depende

const shr = await project.loadShrinkwrap()

t.notOk(shr.packages['/dep-of-pkg-with-1-dep/100.0.0']['optional'])
t.notOk(shr.packages['/dep-of-pkg-with-1-dep/100.0.0']['optional'], 'package is not marked as optional')
})
5 changes: 5 additions & 0 deletions typings/local.d.ts
Expand Up @@ -253,3 +253,8 @@ declare module 'dint' {
const anything: any;
export = anything;
}

declare module 'is-subdir' {
const anything: any;
export = anything;
}

0 comments on commit c69bea6

Please sign in to comment.