Skip to content

Commit

Permalink
Warn for missing bundledDependencies (#4046)
Browse files Browse the repository at this point in the history
* [#886] Added test for missing bundledDependencies

* Requested changes for PR 4046
  • Loading branch information
rally25rs authored and arcanis committed Jul 28, 2017
1 parent 8a1dfee commit 7092964
Show file tree
Hide file tree
Showing 26 changed files with 63 additions and 6 deletions.
30 changes: 28 additions & 2 deletions __tests__/commands/install/integration.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import {run as check} from '../../../src/cli/commands/check.js';
import * as constants from '../../../src/constants.js';
import * as reporters from '../../../src/reporters/index.js';
import {parse} from '../../../src/lockfile/wrapper.js';
import {Install} from '../../../src/cli/commands/install.js';
import {Install, run as install} from '../../../src/cli/commands/install.js';
import Lockfile from '../../../src/lockfile/wrapper.js';
import * as fs from '../../../src/util/fs.js';
import {getPackageVersion, explodeLockfile, runInstall, createLockfile} from '../_helpers.js';
import {getPackageVersion, explodeLockfile, runInstall, createLockfile, run as buildRun} from '../_helpers.js';

jasmine.DEFAULT_TIMEOUT_INTERVAL = 150000;

Expand Down Expand Up @@ -992,3 +992,29 @@ test.concurrent('top level patterns should match after install', (): Promise<voi
expect(integrityError).toBe(false);
});
});

test.concurrent('warns for missing bundledDependencies', (): Promise<void> => {
const fixturesLoc = path.join(__dirname, '..', '..', 'fixtures', 'install');

return buildRun(
reporters.BufferReporter,
fixturesLoc,
async (args, flags, config, reporter): Promise<void> => {
await install(config, reporter, flags, args);

const output = reporter.getBuffer();
const warnings = output.filter(entry => entry.type === 'warning');

expect(
warnings.some(warning => {
return (
warning.data.toString().indexOf(reporter.lang('missingBundledDependency', '[email protected]', 'tap-consumer')) > -1
);
}),
).toEqual(true);
},
[],
{},
'missing-bundled-dep',
);
});
5 changes: 5 additions & 0 deletions __tests__/fixtures/install/missing-bundled-dep/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"dependencies": {
"tap": "0.3.1"
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
33 changes: 29 additions & 4 deletions src/package-linker.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,18 @@ export default class PackageLinker {
if (pkg.bundleDependencies) {
for (const depName of pkg.bundleDependencies) {
const loc = path.join(this.config.generateHardModulePath(ref), this.config.getFolder(pkg), depName);
try {
const dep = await this.config.readManifest(loc, remote.registry);

const dep = await this.config.readManifest(loc, remote.registry);

if (dep.bin && Object.keys(dep.bin).length) {
deps.push({dep, loc});
if (dep.bin && Object.keys(dep.bin).length) {
deps.push({dep, loc});
}
} catch (ex) {
if (ex.code !== 'ENOENT') {
throw ex;
}
// intentionally ignoring ENOENT error.
// bundledDependency either does not exist or does not contain a package.json
}
}
}
Expand Down Expand Up @@ -380,6 +387,10 @@ export default class PackageLinker {
linkBinConcurrency,
);
}

for (const [, {pkg}] of flatTree) {
await this._warnForMissingBundledDependencies(pkg);
}
}

determineTopLevelBinLinks(flatTree: HoistManifestTuples): Array<[string, Manifest]> {
Expand Down Expand Up @@ -432,6 +443,20 @@ export default class PackageLinker {
return range === '*' || satisfiesWithPreleases(version, range, this.config.looseSemver);
}

async _warnForMissingBundledDependencies(pkg: Manifest): Promise<void> {
const ref = pkg._reference;

if (pkg.bundleDependencies) {
for (const depName of pkg.bundleDependencies) {
const loc = path.join(this.config.generateHardModulePath(ref), this.config.getFolder(pkg), depName);
if (!await fs.exists(loc)) {
const pkgHuman = `${pkg.name}@${pkg.version}`;
this.reporter.warn(this.reporter.lang('missingBundledDependency', pkgHuman, depName));
}
}
}
}

async init(
patterns: Array<string>,
workspaceLayout?: WorkspaceLayout,
Expand Down
1 change: 1 addition & 0 deletions src/reporters/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ const messages = {

unmetPeer: '$0 has unmet peer dependency $1.',
incorrectPeer: '$0 has incorrect peer dependency $1.',
missingBundledDependency: '$0 is missing a bundled dependency $1. This should be reported to the package maintainer.',

savedNewDependency: 'Saved 1 new dependency.',
savedNewDependencies: 'Saved $0 new dependencies.',
Expand Down

0 comments on commit 7092964

Please sign in to comment.