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.

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

refactor(npm): skip internal packages earlier #6639

Merged
merged 3 commits into from Jun 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/manager/common.ts
Expand Up @@ -76,7 +76,7 @@ export interface PackageFile<T = Record<string, any>>
extends NpmLockFiles,
ManagerData<T> {
hasYarnWorkspaces?: boolean;
internalPackages?: string[];
internalPackages?: string[]; // TODO: remove
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only cached extracts should have this

compatibility?: Record<string, string>;
datasource?: string;
registryUrls?: string[];
Expand Down
57 changes: 29 additions & 28 deletions lib/manager/npm/extract/__snapshots__/monorepo.spec.ts.snap
Expand Up @@ -3,19 +3,43 @@
exports[`manager/npm/extract .extractPackageFile() uses lerna package settings 1`] = `
Array [
Object {
"internalPackages": Array [
"@org/a",
"@org/b",
"deps": Array [
Object {
"depName": "@org/a",
"skipReason": "internal-package",
},
Object {
"depName": "@org/b",
"skipReason": "internal-package",
},
Object {
"depName": "@org/c",
},
Object {
"depName": "foo",
},
],
"lernaDir": ".",
"lernaPackages": Array [
"packages/*",
],
"packageFile": "package.json",
"packages": Array [
"packages/*",
],
},
Object {
"internalPackages": Array [
"@org/b",
"deps": Array [
Object {
"depName": "@org/b",
"skipReason": "internal-package",
},
Object {
"depName": "@org/c",
},
Object {
"depName": "bar",
},
],
"lernaClient": undefined,
"lernaDir": ".",
Expand All @@ -25,9 +49,6 @@ Array [
"yarnLock": undefined,
},
Object {
"internalPackages": Array [
"@org/a",
],
"lernaClient": undefined,
"lernaDir": ".",
"npmLock": undefined,
Expand All @@ -41,10 +62,6 @@ Array [
exports[`manager/npm/extract .extractPackageFile() uses yarn workspaces package settings with lerna 1`] = `
Array [
Object {
"internalPackages": Array [
"@org/a",
"@org/b",
],
"lernaClient": "yarn",
"lernaDir": ".",
"lernaPackages": Array [
Expand All @@ -56,9 +73,6 @@ Array [
],
},
Object {
"internalPackages": Array [
"@org/b",
],
"lernaClient": "yarn",
"lernaDir": ".",
"npmLock": undefined,
Expand All @@ -67,9 +81,6 @@ Array [
"yarnLock": undefined,
},
Object {
"internalPackages": Array [
"@org/a",
],
"lernaClient": "yarn",
"lernaDir": ".",
"npmLock": undefined,
Expand All @@ -83,18 +94,11 @@ Array [
exports[`manager/npm/extract .extractPackageFile() uses yarn workspaces package settings without lerna 1`] = `
Array [
Object {
"internalPackages": Array [
"@org/a",
"@org/b",
],
"packageFile": "package.json",
"yarnWorkspacesPackages": "packages/*",
},
Object {
"hasYarnWorkspaces": true,
"internalPackages": Array [
"@org/b",
],
"lernaClient": undefined,
"lernaDir": undefined,
"npmLock": undefined,
Expand All @@ -103,9 +107,6 @@ Array [
"yarnLock": "yarn.lock",
},
Object {
"internalPackages": Array [
"@org/a",
],
"lernaClient": undefined,
"lernaDir": undefined,
"npmLock": undefined,
Expand Down
29 changes: 26 additions & 3 deletions lib/manager/npm/extract/monorepo.spec.ts
Expand Up @@ -8,10 +8,36 @@ describe('manager/npm/extract', () => {
packageFile: 'package.json',
lernaDir: '.',
lernaPackages: ['packages/*'],
packages: ['packages/*'],
deps: [
{
depName: '@org/a',
},
{
depName: '@org/b',
},
{
depName: '@org/c',
},
{
depName: 'foo',
},
],
},
{
packageFile: 'packages/a/package.json',
packageJsonName: '@org/a',
deps: [
{
depName: '@org/b',
},
{
depName: '@org/c',
},
{
depName: 'bar',
},
],
},
{
packageFile: 'packages/b/package.json',
Expand All @@ -21,7 +47,6 @@ describe('manager/npm/extract', () => {
detectMonorepos(packageFiles);
expect(packageFiles).toMatchSnapshot();
expect(packageFiles[1].lernaDir).toEqual('.');
expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
});
it('uses yarn workspaces package settings with lerna', () => {
const packageFiles = [
Expand All @@ -44,7 +69,6 @@ describe('manager/npm/extract', () => {
detectMonorepos(packageFiles);
expect(packageFiles).toMatchSnapshot();
expect(packageFiles[1].lernaDir).toEqual('.');
expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
});
it('uses yarn workspaces package settings without lerna', () => {
const packageFiles = [
Expand All @@ -64,7 +88,6 @@ describe('manager/npm/extract', () => {
];
detectMonorepos(packageFiles);
expect(packageFiles).toMatchSnapshot();
expect((packageFiles[1] as any).internalPackages).toEqual(['@org/b']);
});
});
});
18 changes: 12 additions & 6 deletions lib/manager/npm/extract/monorepo.ts
Expand Up @@ -4,6 +4,7 @@ import is from '@sindresorhus/is';
import minimatch from 'minimatch';
import upath from 'upath';
import { logger } from '../../../logger';
import { SkipReason } from '../../../types';
import { PackageFile } from '../../common';

function matchesAnyPattern(val: string, patterns: string[]): boolean {
Expand Down Expand Up @@ -40,22 +41,27 @@ export function detectMonorepos(packageFiles: Partial<PackageFile>[]): void {
const internalPackageFiles = packageFiles.filter((sp) =>
matchesAnyPattern(path.dirname(sp.packageFile), internalPackagePatterns)
);
const internalPackages = internalPackageFiles
const internalPackageNames = internalPackageFiles
.map((sp) => sp.packageJsonName)
.filter(Boolean);
// add all names to main package.json
p.internalPackages = internalPackages;
p.deps?.forEach((dep) => {
if (internalPackageNames.includes(dep.depName)) {
dep.skipReason = SkipReason.InternalPackage; // eslint-disable-line no-param-reassign
}
});
for (const subPackage of internalPackageFiles) {
subPackage.internalPackages = internalPackages.filter(
(name) => name !== subPackage.packageJsonName
);
subPackage.lernaDir = lernaDir;
subPackage.lernaClient = lernaClient;
subPackage.yarnLock = subPackage.yarnLock || yarnLock;
subPackage.npmLock = subPackage.npmLock || npmLock;
if (subPackage.yarnLock) {
subPackage.hasYarnWorkspaces = !!yarnWorkspacesPackages;
}
subPackage.deps?.forEach((dep) => {
if (internalPackageNames.includes(dep.depName)) {
dep.skipReason = SkipReason.InternalPackage; // eslint-disable-line no-param-reassign
}
});
}
}
}
Expand Down
Expand Up @@ -66,11 +66,6 @@ Object {
"skipReason": "ignored",
"updates": Array [],
},
Object {
"depName": "zzzz",
"skipReason": "internal-package",
"updates": Array [],
},
Object {
"depName": "foo",
"skipReason": "disabled",
Expand All @@ -82,9 +77,6 @@ Object {
"updates": Array [],
},
],
"internalPackages": Array [
"zzzz",
],
"packageFile": "package.json",
},
],
Expand Down
8 changes: 1 addition & 7 deletions lib/workers/repository/process/fetch.spec.ts
Expand Up @@ -39,24 +39,18 @@ describe('workers/repository/process/fetch', () => {
packageFile: 'package.json',
deps: [
{ depName: 'abcd' },
{ depName: 'zzzz' },
{ depName: 'foo' },
{ depName: 'skipped', skipReason: 'some-reason' as never },
],
internalPackages: ['zzzz'],
},
],
};
await fetchUpdates(config, packageFiles);
expect(packageFiles).toMatchSnapshot();
expect(packageFiles.npm[0].deps[0].skipReason).toEqual('ignored');
expect(packageFiles.npm[0].deps[0].updates).toHaveLength(0);
expect(packageFiles.npm[0].deps[1].skipReason).toEqual(
'internal-package'
);
expect(packageFiles.npm[0].deps[1].skipReason).toEqual('disabled');
expect(packageFiles.npm[0].deps[1].updates).toHaveLength(0);
expect(packageFiles.npm[0].deps[2].skipReason).toEqual('disabled');
expect(packageFiles.npm[0].deps[2].updates).toHaveLength(0);
});
it('fetches updates', async () => {
config.rangeStrategy = 'auto';
Expand Down
10 changes: 2 additions & 8 deletions lib/workers/repository/process/fetch.ts
Expand Up @@ -33,14 +33,8 @@ async function fetchDepUpdates(
if (depConfig.ignoreDeps.includes(depName)) {
logger.debug({ dependency: dep.depName }, 'Dependency is ignored');
dep.skipReason = SkipReason.Ignored;
} else if (
depConfig.internalPackages &&
depConfig.internalPackages.includes(depName)
) {
logger.debug(
{ dependency: dep.depName },
'Dependency is ignored due to being internal'
);
} else if (depConfig.internalPackages?.includes(depName)) {
// istanbul ignore next
dep.skipReason = SkipReason.InternalPackage;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Need to keep this in for cached extracts

} else if (depConfig.enabled === false) {
logger.debug({ dependency: dep.depName }, 'Dependency is disabled');
Expand Down