github-mirrors/kibana
1
0
Fork 0
mirror of https://github.com/elastic/kibana.git synced 2025-06-27 18:51:07 -04:00
Code Issues Projects Releases Packages Wiki Activity

switch out chokidar for @parcel/watcher in dev cli (#148924)

Browse source 
After the recent changes to limit the dev-cli watcher to relevant
packages, the watcher started logging tons of unnecessary changes, and
in some cases breaking based on the state of the repo. I have seen this
happen with Chokidar before, and I'm not convinced we'll be able to fix
it, so instead I decided to swap it out with `@parcel/watcher`, which is
a conceptually simpler implementation that automatically batches changes
and watches an entire directory, rather than tons of unique
directories/files.

This new implementation is conceptually simpler, and because of the
design of the `@parcel/watcher` module I was pushed to reuse the
`RepoSourceClassifier` to determine if we should restart the server
based on a specific change. This means we now have a single source of
truth for test files and the like (the classifier will tell us if a file
is a test file, regardless of where it exists in the repo).

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
Spencer 2023-01-13 17:42:13 -06:00 • committed by GitHub
parent a94a1b620e
commit 99013bdab8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 107 additions and 538 deletions
Download patch file Download diff file Expand all files Collapse all files

2
packages/kbn-cli-dev-mode/README.mdx
View file

@ -23,7 +23,7 @@ objects.
## `Watcher`
The `Watcher` manages a [chokidar](https://github.com/paulmillr/chokidar) instance to watch the
The `Watcher` manages a [@parcel/watcher](https://github.com/parcel-bundler/watcher) instance to watch the
server files, logs about file changes observed and provides an observable to the `DevServer` via
its `serverShouldRestart$()` method.

15
packages/kbn-cli-dev-mode/src/cli_dev_mode.test.ts
View file

@ -35,13 +35,6 @@ const { BasePathProxyServer } = jest.requireMock('./base_path_proxy_server');
jest.mock('@kbn/ci-stats-reporter');
const { CiStatsReporter } = jest.requireMock('@kbn/ci-stats-reporter');
jest.mock('./get_server_watch_paths', () => ({
getServerWatchPaths: jest.fn(() => ({
watchPaths: ['<mock watch paths>'],
ignorePaths: ['<mock ignore paths>'],
})),
}));
const mockBasePathProxy = {
targetPort: 9999,
basePath: '/foo/bar',
@ -142,15 +135,9 @@ it('passes correct args to sub-classes', () => {
Array [
Array [
Object {
"cwd": <absolute path>,
"enabled": true,
"ignore": Array [
"<mock ignore paths>",
],
"log": <TestLog>,
"paths": Array [
"<mock watch paths>",
],
"repoRoot": <absolute path>,
},
],
]

11
packages/kbn-cli-dev-mode/src/cli_dev_mode.ts
View file

@ -31,7 +31,6 @@ import { DevServer } from './dev_server';
import { Watcher } from './watcher';
import { BasePathProxyServer } from './base_path_proxy_server';
import { shouldRedirectFromOldBasePath } from './should_redirect_from_old_base_path';
import { getServerWatchPaths } from './get_server_watch_paths';
import { CliDevConfig } from './config';
// signal that emits undefined once a termination signal has been sent
@ -114,18 +113,10 @@ export class CliDevMode {
this.basePathProxy = new BasePathProxyServer(this.log, config.http, config.dev);
}
const { watchPaths, ignorePaths } = getServerWatchPaths({
runExamples: cliArgs.runExamples,
pluginPaths: config.plugins.additionalPluginPaths,
pluginScanDirs: config.plugins.pluginSearchPaths,
});
this.watcher = new Watcher({
enabled: !!cliArgs.watch,
log: this.log,
cwd: REPO_ROOT,
paths: watchPaths,
ignore: ignorePaths,
repoRoot: REPO_ROOT,
});
this.devServer = new DevServer({

140
packages/kbn-cli-dev-mode/src/get_server_watch_paths.test.ts
View file

@ -1,140 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import Path from 'path';
import { createAbsolutePathSerializer } from '@kbn/jest-serializers';
import { REPO_ROOT } from '@kbn/repo-info';
import type { KibanaPackageType } from '@kbn/repo-packages';
const TYPES = Object.keys(
(() => {
const asObj: { [k in KibanaPackageType]: true } = {
'functional-tests': true,
'plugin-browser': true,
'plugin-server': true,
'shared-browser': true,
'shared-common': true,
'shared-scss': true,
'shared-server': true,
'test-helper': true,
};
return asObj;
})()
);
import { getServerWatchPaths } from './get_server_watch_paths';
jest.mock('@kbn/repo-packages', () => ({
getPackages: jest.fn(),
getPluginPackagesFilter: jest.fn().mockReturnValue(() => true),
}));
const mockGetPluginPackagesFilter = jest.requireMock('@kbn/repo-packages').getPluginPackagesFilter;
const mockGetPackages = jest.requireMock('@kbn/repo-packages').getPackages;
expect.addSnapshotSerializer(createAbsolutePathSerializer());
it('produces the right watch and ignore list', () => {
mockGetPackages.mockReturnValue(
TYPES.flatMap((type) => ({
isPlugin: type.startsWith('plugin-'),
directory: Path.resolve(REPO_ROOT, 'packages', type),
manifest: {
type,
},
}))
);
const { watchPaths, ignorePaths } = getServerWatchPaths({
runExamples: false,
pluginPaths: [Path.resolve(REPO_ROOT, 'x-pack/test/plugin_functional/plugins/resolver_test')],
pluginScanDirs: [
Path.resolve(REPO_ROOT, 'src/plugins'),
Path.resolve(REPO_ROOT, 'test/plugin_functional/plugins'),
Path.resolve(REPO_ROOT, 'x-pack/plugins'),
],
});
expect(watchPaths).toMatchInlineSnapshot(`
Array [
<absolute path>/src/core,
<absolute path>/config,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test,
<absolute path>/src/plugins,
<absolute path>/test/plugin_functional/plugins,
<absolute path>/x-pack/plugins,
<absolute path>/packages/plugin-server,
<absolute path>/packages/shared-common,
<absolute path>/packages/shared-server,
]
`);
expect(ignorePaths).toMatchInlineSnapshot(`
Array [
/\\[\\\\\\\\\\\\/\\]\\(\\\\\\.\\.\\*\\|node_modules\\|bower_components\\|target\\|public\\|__\\[a-z0-9_\\]\\+__\\|coverage\\)\\(\\[\\\\\\\\\\\\/\\]\\|\\$\\)/,
/\\\\\\.\\(test\\|spec\\)\\\\\\.\\(js\\|ts\\|tsx\\)\\$/,
/\\\\\\.\\(md\\|sh\\|txt\\)\\$/,
/debug\\\\\\.log\\$/,
<absolute path>/src/plugins/*/test/**,
<absolute path>/src/plugins/*/integration_tests/**,
<absolute path>/src/plugins/*/build/**,
<absolute path>/src/plugins/*/target/**,
<absolute path>/src/plugins/*/scripts/**,
<absolute path>/src/plugins/*/docs/**,
<absolute path>/test/plugin_functional/plugins/*/test/**,
<absolute path>/test/plugin_functional/plugins/*/integration_tests/**,
<absolute path>/test/plugin_functional/plugins/*/build/**,
<absolute path>/test/plugin_functional/plugins/*/target/**,
<absolute path>/test/plugin_functional/plugins/*/scripts/**,
<absolute path>/test/plugin_functional/plugins/*/docs/**,
<absolute path>/x-pack/plugins/*/test/**,
<absolute path>/x-pack/plugins/*/integration_tests/**,
<absolute path>/x-pack/plugins/*/build/**,
<absolute path>/x-pack/plugins/*/target/**,
<absolute path>/x-pack/plugins/*/scripts/**,
<absolute path>/x-pack/plugins/*/docs/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/test/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/integration_tests/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/build/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/target/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/scripts/**,
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test/docs/**,
<absolute path>/x-pack/plugins/screenshotting/chromium,
<absolute path>/x-pack/plugins/security_solution/cypress,
<absolute path>/x-pack/plugins/apm/scripts,
<absolute path>/x-pack/plugins/apm/ftr_e2e,
<absolute path>/x-pack/plugins/canvas/canvas_plugin_src,
<absolute path>/x-pack/plugins/cases/server/scripts,
<absolute path>/x-pack/plugins/lists/scripts,
<absolute path>/x-pack/plugins/lists/server/scripts,
<absolute path>/x-pack/plugins/security_solution/scripts,
<absolute path>/x-pack/plugins/security_solution/server/lib/detection_engine/scripts,
<absolute path>/x-pack/plugins/synthetics/e2e,
<absolute path>/x-pack/plugins/ux/e2e,
<absolute path>/x-pack/plugins/observability/e2e,
]
`);
expect(mockGetPluginPackagesFilter.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Object {
"examples": false,
"parentDirs": Array [
<absolute path>/src/plugins,
<absolute path>/test/plugin_functional/plugins,
<absolute path>/x-pack/plugins,
],
"paths": Array [
<absolute path>/x-pack/test/plugin_functional/plugins/resolver_test,
],
},
],
]
`);
});

94
packages/kbn-cli-dev-mode/src/get_server_watch_paths.ts
View file

@ -1,94 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import Path from 'path';
import { REPO_ROOT } from '@kbn/repo-info';
import { getPackages, getPluginPackagesFilter } from '@kbn/repo-packages';
interface Options {
runExamples: boolean;
pluginPaths: string[];
pluginScanDirs: string[];
}
export type WatchPaths = ReturnType<typeof getServerWatchPaths>;
export function getServerWatchPaths(opts: Options) {
const fromRoot = (p: string) => Path.resolve(REPO_ROOT, p);
const pluginInternalDirsIgnore = opts.pluginScanDirs
.map((scanDir) => Path.resolve(scanDir, '*'))
.concat(opts.pluginPaths)
.reduce(
(acc: string[], path) => [
...acc,
Path.resolve(path, 'test/**'),
Path.resolve(path, 'integration_tests/**'),
Path.resolve(path, 'build/**'),
Path.resolve(path, 'target/**'),
Path.resolve(path, 'scripts/**'),
Path.resolve(path, 'docs/**'),
],
[]
);
function getServerPkgDirs() {
const pluginFilter = getPluginPackagesFilter({
examples: opts.runExamples,
paths: opts.pluginPaths,
parentDirs: opts.pluginScanDirs,
});
return getPackages(REPO_ROOT).flatMap((p) => {
if (p.isPlugin) {
return pluginFilter(p) && p.manifest.type === 'plugin-server' ? p.directory : [];
}
return p.manifest.type === 'shared-common' || p.manifest.type === 'shared-server'
? p.directory
: [];
});
}
const watchPaths = Array.from(
new Set([
fromRoot('src/core'),
fromRoot('config'),
...opts.pluginPaths.map((path) => Path.resolve(path)),
...opts.pluginScanDirs.map((path) => Path.resolve(path)),
...getServerPkgDirs(),
])
);
const ignorePaths = [
/[\\\/](\..*|node_modules|bower_components|target|public|__[a-z0-9_]+__|coverage)([\\\/]|$)/,
/\.(test|spec)\.(js|ts|tsx)$/,
/\.(md|sh|txt)$/,
/debug\.log$/,
...pluginInternalDirsIgnore,
fromRoot('x-pack/plugins/screenshotting/chromium'),
fromRoot('x-pack/plugins/security_solution/cypress'),
fromRoot('x-pack/plugins/apm/scripts'),
fromRoot('x-pack/plugins/apm/ftr_e2e'), // prevents restarts for APM cypress tests
fromRoot('x-pack/plugins/canvas/canvas_plugin_src'), // prevents server from restarting twice for Canvas plugin changes,
fromRoot('x-pack/plugins/cases/server/scripts'),
fromRoot('x-pack/plugins/lists/scripts'),
fromRoot('x-pack/plugins/lists/server/scripts'),
fromRoot('x-pack/plugins/security_solution/scripts'),
fromRoot('x-pack/plugins/security_solution/server/lib/detection_engine/scripts'),
fromRoot('x-pack/plugins/synthetics/e2e'),
fromRoot('x-pack/plugins/ux/e2e'),
fromRoot('x-pack/plugins/observability/e2e'),
];
return {
watchPaths,
ignorePaths,
};
}

207
packages/kbn-cli-dev-mode/src/watcher.test.ts
View file

@ -1,207 +0,0 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/
import { EventEmitter } from 'events';
import * as Rx from 'rxjs';
import { materialize, toArray } from 'rxjs/operators';
import { TestLog } from './log';
import { Watcher, Options } from './watcher';
class MockChokidar extends EventEmitter {
close = jest.fn();
}
let mockChokidar: MockChokidar | undefined;
jest.mock('chokidar');
const chokidar = jest.requireMock('chokidar');
function isMock(mock: MockChokidar | undefined): asserts mock is MockChokidar {
expect(mock).toBeInstanceOf(MockChokidar);
}
chokidar.watch.mockImplementation(() => {
mockChokidar = new MockChokidar();
return mockChokidar;
});
const subscriptions: Rx.Subscription[] = [];
const run = (watcher: Watcher) => {
const subscription = watcher.run$.subscribe({
error(e) {
throw e;
},
});
subscriptions.push(subscription);
return subscription;
};
const log = new TestLog();
const defaultOptions: Options = {
enabled: true,
log,
paths: ['foo.js', 'bar.js'],
ignore: [/^f/],
cwd: '/app/repo',
};
afterEach(() => {
jest.clearAllMocks();
if (mockChokidar) {
mockChokidar.removeAllListeners();
mockChokidar = undefined;
}
for (const sub of subscriptions) {
sub.unsubscribe();
}
subscriptions.length = 0;
log.messages.length = 0;
});
it('completes restart streams immediately when disabled', () => {
const watcher = new Watcher({
...defaultOptions,
enabled: false,
});
const restart$ = new Rx.BehaviorSubject<void>(undefined);
subscriptions.push(watcher.serverShouldRestart$().subscribe(restart$));
run(watcher);
expect(restart$.isStopped).toBe(true);
});
it('calls chokidar.watch() with expected arguments', () => {
const watcher = new Watcher(defaultOptions);
expect(chokidar.watch).not.toHaveBeenCalled();
run(watcher);
expect(chokidar.watch.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
Array [
"foo.js",
"bar.js",
],
Object {
"cwd": "/app/repo",
"ignored": Array [
/\\^f/,
],
},
],
]
`);
});
it('closes chokidar watcher when unsubscribed', () => {
const sub = run(new Watcher(defaultOptions));
isMock(mockChokidar);
expect(mockChokidar.close).not.toHaveBeenCalled();
sub.unsubscribe();
expect(mockChokidar.close).toHaveBeenCalledTimes(1);
});
it('rethrows chokidar errors', async () => {
const watcher = new Watcher(defaultOptions);
const promise = Rx.firstValueFrom(watcher.run$.pipe(materialize(), toArray()));
isMock(mockChokidar);
mockChokidar.emit('error', new Error('foo bar'));
const notifications = await promise;
expect(notifications).toMatchInlineSnapshot(`
Array [
Notification {
"error": [Error: foo bar],
"hasValue": false,
"kind": "E",
"value": undefined,
},
]
`);
});
it('logs the count of add events after the ready event', () => {
run(new Watcher(defaultOptions));
isMock(mockChokidar);
mockChokidar.emit('add');
mockChokidar.emit('add');
mockChokidar.emit('add');
mockChokidar.emit('add');
mockChokidar.emit('ready');
expect(log.messages).toMatchInlineSnapshot(`
Array [
Object {
"args": Array [
"watching for changes",
"(4 files)",
],
"type": "good",
},
]
`);
});
it('buffers subsequent changes before logging and notifying serverShouldRestart$', async () => {
const watcher = new Watcher(defaultOptions);
const history: any[] = [];
subscriptions.push(
watcher
.serverShouldRestart$()
.pipe(materialize())
.subscribe((n) => history.push(n))
);
run(watcher);
expect(history).toMatchInlineSnapshot(`Array []`);
isMock(mockChokidar);
mockChokidar.emit('ready');
mockChokidar.emit('all', ['add', 'foo.js']);
mockChokidar.emit('all', ['add', 'bar.js']);
mockChokidar.emit('all', ['delete', 'bar.js']);
await new Promise((resolve) => setTimeout(resolve, 1000));
expect(log.messages).toMatchInlineSnapshot(`
Array [
Object {
"args": Array [
"watching for changes",
"(0 files)",
],
"type": "good",
},
Object {
"args": Array [
"restarting server",
"due to changes in
- \\"foo.js\\"
- \\"bar.js\\"",
],
"type": "warn",
},
]
`);
expect(history).toMatchInlineSnapshot(`
Array [
Notification {
"error": undefined,
"hasValue": true,
"kind": "N",
"value": undefined,
},
]
`);
});

136
packages/kbn-cli-dev-mode/src/watcher.ts
View file

@ -7,44 +7,48 @@
*/
import * as Rx from 'rxjs';
import {
map,
tap,
takeUntil,
count,
share,
buffer,
debounceTime,
ignoreElements,
} from 'rxjs/operators';
import Chokidar from 'chokidar';
import Pw from '@parcel/watcher';
import { REPO_ROOT } from '@kbn/repo-info';
import { RepoSourceClassifier } from '@kbn/repo-source-classifier';
import { ImportResolver } from '@kbn/import-resolver';
import { makeMatcher } from '@kbn/picomatcher';
import { Log } from './log';
const packageMatcher = makeMatcher(['**/*', '!**/.*']);
/**
* Any non-package code must match this in order to trigger a restart
*/
const nonPackageMatcher = makeMatcher([
'src/**',
'!src/{dev,fixtures}/**',
'x-pack/plugins/**',
'!x-pack/plugins/screenshotting/chromium',
'!x-pack/plugins/canvas/canvas_plugin_src',
]);
export interface Options {
enabled: boolean;
log: Log;
paths: string[];
ignore: Array<string | RegExp>;
cwd: string;
repoRoot: string;
}
export class Watcher {
public readonly enabled: boolean;
private readonly log: Log;
private readonly paths: string[];
private readonly ignore: Array<string | RegExp>;
private readonly cwd: string;
private readonly repoRoot: string;
private readonly classifier: RepoSourceClassifier;
private readonly restart$ = new Rx.Subject<void>();
private readonly resolver: ImportResolver;
constructor(options: Options) {
this.enabled = !!options.enabled;
this.log = options.log;
this.paths = options.paths;
this.ignore = options.ignore;
this.cwd = options.cwd;
this.repoRoot = options.repoRoot;
this.resolver = ImportResolver.create(REPO_ROOT);
this.classifier = new RepoSourceClassifier(this.resolver);
}
run$ = new Rx.Observable((subscriber) => {
@ -54,54 +58,56 @@ export class Watcher {
return;
}
const chokidar = Chokidar.watch(this.paths, {
cwd: this.cwd,
ignored: this.ignore,
});
const fire = (repoRel: string) => {
this.log.warn(`restarting server`, `due to changes in ${repoRel}`);
this.restart$.next();
};
subscriber.add(() => {
chokidar.close();
});
Pw.subscribe(
this.repoRoot,
(error, events) => {
if (error) {
subscriber.error(error);
return;
}
const error$ = Rx.fromEvent(chokidar, 'error').pipe(
map((error) => {
throw error;
})
);
for (const event of events) {
const pkg = this.resolver.getPackageManifestForPath(event.path);
const init$ = Rx.fromEvent(chokidar, 'add').pipe(
takeUntil(Rx.fromEvent(chokidar, 'ready')),
count(),
tap((fileCount) => {
this.log.good('watching for changes', `(${fileCount} files)`);
})
);
// ignore changes in any devOnly package, these can't power the server so we can ignore them
if (pkg?.devOnly) {
return;
}
const change$ = Rx.fromEvent<[string, string]>(chokidar, 'all').pipe(
map(([, path]) => path),
share()
);
subscriber.add(
Rx.merge(
error$,
Rx.concat(
init$,
change$.pipe(
buffer(change$.pipe(debounceTime(50))),
map((changes) => {
const paths = Array.from(new Set(changes));
const prefix = paths.length > 1 ? '\n - ' : ' ';
const fileList = paths.reduce((list, file) => `${list || ''}${prefix}"${file}"`, '');
this.log.warn(`restarting server`, `due to changes in${fileList}`);
this.restart$.next();
})
)
)
)
.pipe(ignoreElements())
.subscribe(subscriber)
const result = this.classifier.classify(event.path);
if (result.type === 'common package' || result.type === 'server package') {
return packageMatcher(result.repoRel) && fire(result.repoRel);
}
if (result.type === 'non-package') {
return nonPackageMatcher(result.repoRel) && fire(result.repoRel);
}
}
},
{
// some basic high-level ignore statements. Additional filtering is done above
// before paths are passed to `fire()`, using the RepoSourceClassifier mostly
ignore: [
'**/{node_modules,target,public,coverage,__*__}/**',
'**/*.{test,spec,story,stories}.*',
'**/*.{md,sh,txt}',
'**/debug.log',
],
}
).then(
(sub) => {
this.log.good('watching server files for changes');
subscriber.add(() => {
sub.unsubscribe();
});
},
(error) => {
subscriber.error(error);
}
);
// complete state subjects when run$ completes

4
packages/kbn-cli-dev-mode/tsconfig.json
View file

@ -21,7 +21,9 @@
"@kbn/jest-serializers",
"@kbn/stdio-dev-helpers",
"@kbn/tooling-log",
"@kbn/repo-packages",
"@kbn/repo-source-classifier",
"@kbn/import-resolver",
"@kbn/picomatcher",
],
"exclude": [
"target/**/*",