Skip to content

Commit

Permalink
fix: flatten dependencies by alias (#47)
Browse files Browse the repository at this point in the history
* fix: flatten by alias

* fix: rename field, other fixes

* fix: more bug fixes

* fix: make all tests run, write a (failing) test for pruning

* fix: reflatten after pruning

* fix: do not log installation/removal of hoisted aliases

* fix: follow review comments

* fix: use muteLogs instead of skipLogs
  • Loading branch information
etamponi authored and zkochan committed Feb 25, 2018
1 parent 95ff32d commit 556910e
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 68 deletions.
2 changes: 2 additions & 0 deletions src/api/getContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export type PnpmContext = {
wantedShrinkwrap: Shrinkwrap,
skipped: Set<string>,
pendingBuilds: string[],
hoistedAliases: {[pkgId: string]: string[]}
}

export default async function getContext (
Expand Down Expand Up @@ -104,6 +105,7 @@ export default async function getContext (
existsCurrentShrinkwrap: !!files[2],
skipped: new Set(modules && modules.skipped || []),
pendingBuilds: modules && modules.pendingBuilds || [],
hoistedAliases: modules && modules.hoistedAliases || {},
}
packageJsonLogger.debug({ initial: ctx.pkg })

Expand Down
5 changes: 4 additions & 1 deletion src/api/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ export async function installPkgs (
devDependencies,
})

if (!Object.keys(packagesToInstall).length) {
if (!Object.keys(packagesToInstall).length && !opts.reinstallForFlatten) {
throw new Error('At least one package has to be installed')
}

Expand Down Expand Up @@ -548,7 +548,9 @@ async function installInContext (
sideEffectsCache: opts.sideEffectsCache,
shamefullyFlatten: opts.shamefullyFlatten,
reinstallForFlatten: Boolean(opts.reinstallForFlatten),
hoistedAliases: ctx.hoistedAliases,
})
ctx.hoistedAliases = result.hoistedAliases

ctx.pendingBuilds = ctx.pendingBuilds
.filter(pkgId => !result.removedPkgIds.has(dp.resolve(ctx.wantedShrinkwrap.registry, pkgId)))
Expand Down Expand Up @@ -576,6 +578,7 @@ async function installInContext (
independentLeaves: opts.independentLeaves,
pendingBuilds: ctx.pendingBuilds,
shamefullyFlatten: opts.shamefullyFlatten,
hoistedAliases: ctx.hoistedAliases,
}),
])

Expand Down
13 changes: 6 additions & 7 deletions src/api/prune.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import {PackageJson} from '@pnpm/types'
import path = require('path')
import R = require('ramda')
import getContext from './getContext'
import {PnpmOptions} from '@pnpm/types'
import extendOptions, {
PruneOptions,
StrictPruneOptions,
} from './extendPruneOptions'
import getPkgDirs from '../fs/getPkgDirs'
import {fromDir as readPkgFromDir} from '../fs/readPkg'
import removeOrphanPkgs from './removeOrphanPkgs'
import {
ResolvedDependencies,
prune as pruneShrinkwrap,
} from 'pnpm-shrinkwrap'
import {streamParser} from '@pnpm/logger'
import {installPkgs} from './install'

export async function prune (
maybeOpts: PruneOptions,
Expand Down Expand Up @@ -48,8 +42,13 @@ export async function prune (
storeController: opts.storeController,
pruneStore: true,
bin: opts.bin,
hoistedAliases: ctx.hoistedAliases,
})

if (opts.shamefullyFlatten) {
await installPkgs(prunedShr.specifiers, {...opts, lock: false, reinstallForFlatten: true, update: false})
}

if (reporter) {
streamParser.removeListener('data', reporter)
}
Expand Down
1 change: 1 addition & 0 deletions src/api/rebuild.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ export async function rebuild (maybeOpts: RebuildOptions) {
independentLeaves: opts.independentLeaves,
pendingBuilds: [],
shamefullyFlatten: opts.shamefullyFlatten,
hoistedAliases: ctx.hoistedAliases,
})
}

Expand Down
24 changes: 8 additions & 16 deletions src/api/removeOrphanPkgs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@ import path = require('path')
import * as dp from 'dependency-path'
import {Shrinkwrap, ResolvedPackages} from 'pnpm-shrinkwrap'
import {StoreController} from 'package-store'
import exists = require('path-exists')
import R = require('ramda')
import resolveLinkTarget = require('resolve-link-target')
import removeTopDependency from '../removeTopDependency'
import {dependenciesTypes} from '../getSaveType'
import {statsLogger} from '../loggers'
import getPkgInfoFromShr from '../getPkgInfoFromShr'

export default async function removeOrphanPkgs (
opts: {
Expand All @@ -21,6 +18,7 @@ export default async function removeOrphanPkgs (
shamefullyFlatten: boolean,
storeController: StoreController,
pruneStore?: boolean,
hoistedAliases: {[pkgId: string]: string[]},
}
): Promise<Set<string>> {
const oldPkgs = R.toPairs(R.mergeAll(R.map(depType => opts.oldShrinkwrap[depType], dependenciesTypes)))
Expand Down Expand Up @@ -52,27 +50,21 @@ export default async function removeOrphanPkgs (
if (notDependents.length) {

if (opts.shamefullyFlatten && opts.oldShrinkwrap.packages) {
const packages = opts.oldShrinkwrap.packages
await Promise.all(notDependents.map(async notDependent => {
const pkgShr = packages[dp.relative(opts.oldShrinkwrap.registry, notDependent)]
const pkgInfo = getPkgInfoFromShr(notDependent, pkgShr)
const rootLink = path.join(rootModules, pkgInfo.name)
// rootLink might not exist anymore because it might have been delete earlier
if (await exists(rootLink)) {
const linkTarget = await resolveLinkTarget(rootLink)
const notDependentDir = path.join(rootModules, `.${notDependent}`)
// we only want to remove the root link if it points to a version that we are removing
if (linkTarget.startsWith(notDependentDir)) {
if (opts.hoistedAliases[notDependent]) {
await Promise.all(opts.hoistedAliases[notDependent].map(async alias => {
await removeTopDependency({
name: pkgInfo.name,
name: alias,
dev: false,
optional: false,
}, {
modules: rootModules,
bin: opts.bin
bin: opts.bin,
muteLogs: true
})
}
}))
}
delete opts.hoistedAliases[notDependent]
}))
}

Expand Down
8 changes: 4 additions & 4 deletions src/api/uninstall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export async function uninstallInContext (
shamefullyFlatten: opts.shamefullyFlatten,
storeController: opts.storeController,
bin: opts.bin,
hoistedAliases: ctx.hoistedAliases,
})
ctx.pendingBuilds = ctx.pendingBuilds.filter(pkgId => !removedPkgIds.has(dp.resolve(newShr.registry, pkgId)))
await opts.storeController.close()
Expand All @@ -94,16 +95,15 @@ export async function uninstallInContext (
independentLeaves: opts.independentLeaves,
pendingBuilds: ctx.pendingBuilds,
shamefullyFlatten: opts.shamefullyFlatten,
hoistedAliases: ctx.hoistedAliases,
})
await removeOuterLinks(pkgsToUninstall, path.join(ctx.root, 'node_modules'), {
storePath: ctx.storePath,
bin: opts.bin,
})

if (opts.shamefullyFlatten && R.keys(currentShrinkwrap.specifiers).length > 0) {
await installPkgs(currentShrinkwrap.specifiers, Object.assign({},
opts, {lock: false, reinstallForFlatten: true, update: false}
))
if (opts.shamefullyFlatten) {
await installPkgs(currentShrinkwrap.specifiers, {...opts, lock: false, reinstallForFlatten: true, update: false})
}

logger('summary').info()
Expand Down
1 change: 1 addition & 0 deletions src/fs/modulesController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export type Modules = {
independentLeaves: boolean,
pendingBuilds: string[],
shamefullyFlatten: boolean,
hoistedAliases: {[pkgId: string]: string[]}
}

export async function read (modulesPath: string): Promise<Modules | null> {
Expand Down
72 changes: 40 additions & 32 deletions src/link/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@ export default async function linkPackages (
sideEffectsCache: boolean,
shamefullyFlatten: boolean,
reinstallForFlatten: boolean,
hoistedAliases: {[pkgId: string]: string[]},
}
): Promise<{
linkedPkgsMap: DependencyTreeNodeMap,
wantedShrinkwrap: Shrinkwrap,
currentShrinkwrap: Shrinkwrap,
newDepPaths: string[],
removedPkgIds: Set<string>,
hoistedAliases: {[pkgId: string]: string[]},
}> {
// TODO: decide what kind of logging should be here.
// The `Creating dependency tree` is not good to report in all cases as
Expand All @@ -69,6 +71,7 @@ export default async function linkPackages (
shamefullyFlatten: opts.shamefullyFlatten,
storeController: opts.storeController,
bin: opts.bin,
hoistedAliases: opts.hoistedAliases,
})

let flatResolvedDeps = R.values(pkgsToLink).filter(dep => !opts.skipped.has(dep.id))
Expand Down Expand Up @@ -156,8 +159,9 @@ export default async function linkPackages (
currentShrinkwrap = newCurrentShrinkwrap
}

// Important: shamefullyFlattenTree changes flatResolvedDeps, so keep this at the end
if (opts.shamefullyFlatten && (opts.reinstallForFlatten || newDepPaths.length > 0 || removedPkgIds.size > 0)) {
await shamefullyFlattenTree(flatResolvedDeps, currentShrinkwrap, opts)
opts.hoistedAliases = await shamefullyFlattenTree(flatResolvedDeps, currentShrinkwrap, opts)
}

return {
Expand All @@ -166,6 +170,7 @@ export default async function linkPackages (
currentShrinkwrap,
newDepPaths,
removedPkgIds,
hoistedAliases: opts.hoistedAliases,
}
}

Expand All @@ -180,48 +185,51 @@ async function shamefullyFlattenTree(
pkg: PackageJson,
outdatedPkgs: {[pkgId: string]: string},
},
) {
const pkgNamesExcludedFromFlattening = {}
// first of all, exclude the root packages, as they are already linked
for (let name of R.keys(currentShrinkwrap.specifiers)) {
pkgNamesExcludedFromFlattening[name] = true
}
): Promise<{[alias: string]: string[]}> {
const pkgIdByAlias = {}
const aliasByPkgId: {[pkgId: string]: string[]} = {}

await Promise.all(flatResolvedDeps
// map to make a copy before the sort
// sort by depth and then alphabetically
.map(pkg => pkg).sort((a, b) => {
.sort((a, b) => {
const depthDiff = a.depth - b.depth
return depthDiff === 0 ? a.name.localeCompare(b.name) : depthDiff
})
.filter(pkg => {
const shouldAdd = !pkgNamesExcludedFromFlattening[pkg.name]
// after we add the package to the packages to flatten, exclude subsequent packages with the same name
if (shouldAdd) {
pkgNamesExcludedFromFlattening[pkg.name] = true
// build the alias map and the id map
.map(pkg => {
for (let childAlias of R.keys(pkg.children)) {
// if this alias is in the root dependencies, skip it
if (currentShrinkwrap.specifiers[childAlias]) {
continue
}
// if this alias has already been taken, skip it
if (pkgIdByAlias[childAlias]) {
continue
}
const childId = pkg.children[childAlias]
pkgIdByAlias[childAlias] = childId
if (!aliasByPkgId[childId]) {
aliasByPkgId[childId] = []
}
aliasByPkgId[childId].push(childAlias)
}
return shouldAdd
return pkg
})
.map(async pkg => {
if (opts.dryRun || !(await symlinkDependencyTo(pkg.name, pkg, opts.baseNodeModules)).reused) {
const isDev = opts.pkg.devDependencies && opts.pkg.devDependencies[pkg.name]
const isOptional = opts.pkg.optionalDependencies && opts.pkg.optionalDependencies[pkg.name]
rootLogger.info({
added: {
id: pkg.id,
name: pkg.name,
realName: pkg.name,
version: pkg.version,
latest: opts.outdatedPkgs[pkg.id],
dependencyType: isDev && 'dev' || isOptional && 'optional' || 'prod',
},
})
const pkgAliases = aliasByPkgId[pkg.id]
if (!pkgAliases) {
return
}
// TODO when putting logs back in for hoisted packages, you've to put back the condition inside the map,
// TODO look how it is done in linkPackages
if (!opts.dryRun) {
await Promise.all(pkgAliases.map(async pkgAlias => {
await symlinkDependencyTo(pkgAlias, pkg, opts.baseNodeModules)
}))
}
logStatus({
status: 'installed',
pkgId: pkg.id,
})
}))

return aliasByPkgId
}

function filterShrinkwrap (
Expand Down
17 changes: 10 additions & 7 deletions src/removeTopDependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export default async function removeTopDependency (
dryRun?: boolean,
modules: string,
bin: string,
muteLogs?: boolean,
}
) {
const results = await Promise.all([
Expand All @@ -22,13 +23,15 @@ export default async function removeTopDependency (
])

const uninstalledPkg = results[0]
rootLogger.info({
removed: {
name: dependency.name,
version: uninstalledPkg && uninstalledPkg.version,
dependencyType: dependency.dev && 'dev' || dependency.optional && 'optional' || 'prod'
},
})
if (!opts.muteLogs) {
rootLogger.info({
removed: {
name: dependency.name,
version: uninstalledPkg && uninstalledPkg.version,
dependencyType: dependency.dev && 'dev' || dependency.optional && 'optional' || 'prod'
},
})
}
}

async function removeBins (
Expand Down
Loading

0 comments on commit 556910e

Please sign in to comment.