Filter file changes for watchFilesFromArgs - fixes
MODIFY
packages/@romejs/core/client/Client.ts
packages/@romejs/core/client/Client.ts
@@ -11,7 +11,7 @@ import {
DEFAULT_CLIENT_FLAGS,
} from "../common/types/client";
import ClientRequest, {ClientRequestType} from "./ClientRequest";
-import Server, {ServerOptions} from "../server/Server";
+import Server, {ServerClient, ServerOptions} from "../server/Server";
import {
CLI_SOCKET_PATH,
SOCKET_PATH,
@@ -526,7 +526,11 @@ export default class Client {
async startInternalServer(
opts?: Partial<ServerOptions>,
- ): Promise<BridgeStatusLocal> {
+ ): Promise<{
+ bridge: ServerBridge;
+ server: Server;
+ serverClient: ServerClient;
+ }> {
// Otherwise, start a server inside this process
const server = new Server({
dedicated: false,
@@ -538,13 +542,16 @@ export default class Client {
const bridge = createBridgeFromLocal(ServerBridge, {});
const status: BridgeStatusLocal = {bridge, server, dedicated: false};
- await Promise.all([server.attachToBridge(bridge), this.attachBridge(status)]);
+ const [serverClient] = await Promise.all([
+ server.attachToBridge(bridge),
+ this.attachBridge(status),
+ ]);
this.endEvent.subscribe(async () => {
await server.end();
});
- return status;
+ return {serverClient, bridge, server};
}
async forceStartDaemon(): Promise<ServerBridge> {

MODIFY
packages/@romejs/core/integrationTestHelpers.ts
packages/@romejs/core/integrationTestHelpers.ts
import {TestHelper} from "rome";
import {Client, Server, ServerBridge} from ".";
-import {AbsoluteFilePath, RelativeFilePath, TEMP_PATH} from "@romejs/path";
+import {
+ AbsoluteFilePath,
+ AbsoluteFilePathMap,
+ RelativeFilePath,
+ TEMP_PATH,
+} from "@romejs/path";
import {ClientFlags} from "./common/types/client";
import {JSONObject, stringifyJSON} from "@romejs/codec-json";
import {
@@ -17,6 +22,9 @@ import {Dict} from "@romejs/typescript-helpers";
import {UserConfig} from "./common/userConfig";
import crypto = require("crypto");
import stream = require("stream");
+import ServerRequest from "./server/ServerRequest";
+import {partialServerQueryResponseToFull} from "./server/Server";
+import {PartialServerQueryRequest} from "./common/bridges/ServerBridge";
type IntegrationTestHelper = {
cwd: AbsoluteFilePath;
@@ -27,6 +35,7 @@ type IntegrationTestHelper = {
relative: RelativeFilePath | string,
content: string,
) => Promise<void>;
+ createRequest: (query?: PartialServerQueryRequest) => Promise<ServerRequest>;
};
type IntegrationTestOptions = {
@@ -134,7 +143,7 @@ export function createIntegrationTest(
try {
// Start the server inside of the process
- const {server, bridge} = await client.startInternalServer({
+ const {server, bridge, serverClient} = await client.startInternalServer({
// Only one worker running inside of this process. Don't fork workers.
inbandOnly: true,
// Force cache to be enabled (which will be at our generated folder specified above)
@@ -160,7 +169,18 @@ export function createIntegrationTest(
content: string,
): Promise<void> {
const absolute = projectPath.append(relative);
- await writeFile(absolute, content);
+ await server.writeFiles(
+ new AbsoluteFilePathMap([[absolute, content]]),
+ );
+ },
+ async createRequest(
+ query: PartialServerQueryRequest = {commandName: "unknown"},
+ ) {
+ return new ServerRequest({
+ client: serverClient,
+ query: partialServerQueryResponseToFull(query),
+ server,
+ });
},
},
);

MODIFY
packages/@romejs/core/server/Server.ts
packages/@romejs/core/server/Server.ts
@@ -112,6 +112,28 @@ export type ServerMarker = ServerUnfinishedMarker & {
const disallowedFlagsWhenReviewing: Array<keyof ClientRequestFlags> = ["watch"];
+export function partialServerQueryResponseToFull(
+ partialQuery: PartialServerQueryRequest,
+): ServerQueryRequest {
+ const requestFlags: ClientRequestFlags = {
+ ...DEFAULT_CLIENT_REQUEST_FLAGS,
+ ...partialQuery.requestFlags,
+ };
+
+ return {
+ commandName: partialQuery.commandName,
+ args: partialQuery.args === undefined ? [] : partialQuery.args,
+ noData: partialQuery.noData === true,
+ requestFlags,
+ silent: partialQuery.silent === true || requestFlags.benchmark,
+ terminateWhenIdle: partialQuery.terminateWhenIdle === true,
+ commandFlags: partialQuery.commandFlags === undefined
+ ? {}
+ : partialQuery.commandFlags,
+ cancelToken: partialQuery.cancelToken,
+ };
+}
+
async function validateRequestFlags(
req: ServerRequest,
serverCommand: ServerCommand<Dict<unknown>>,
@@ -401,7 +423,7 @@ export default class Server {
this.logger.list(Array.from(paths, (path) => path.toMarkup()));
}
- let teardown: undefined | (() => void);
+ let teardown: undefined | (() => Promise<void>);
const waitRefresh = new Promise((resolve) => {
const sub = this.refreshFileEvent.subscribe((path) => {
@@ -411,9 +433,7 @@ export default class Server {
}
});
- teardown = () => {
- sub.unsubscribe();
- };
+ teardown = () => sub.unsubscribe();
});
try {
@@ -424,7 +444,7 @@ export default class Server {
await waitRefresh;
} finally {
if (teardown !== undefined) {
- teardown();
+ await teardown();
}
}
}
@@ -462,7 +482,7 @@ export default class Server {
this.workerManager.end();
}
- async attachToBridge(bridge: ServerBridge) {
+ async attachToBridge(bridge: ServerBridge): Promise<ServerClient> {
let profiler: undefined | Profiler;
// If we aren't a dedicated process then we should only expect a single connection
@@ -544,7 +564,7 @@ export default class Server {
`Client version ${client.version} does not match server version ${VERSION}. Goodbye lol.`,
);
client.bridge.end();
- return;
+ return client;
}
bridge.query.subscribe(async (request) => {
@@ -562,6 +582,8 @@ export default class Server {
bridge.endServer.subscribe(async () => this.end());
await this.clientStartEvent.callOptional(client);
+
+ return client;
}
async createClient(bridge: ServerBridge): Promise<ServerClient> {
@@ -706,23 +728,9 @@ export default class Server {
client: ServerClient,
partialQuery: PartialServerQueryRequest,
): Promise<ServerQueryResponse> {
- const requestFlags: ClientRequestFlags = {
- ...DEFAULT_CLIENT_REQUEST_FLAGS,
- ...partialQuery.requestFlags,
- };
-
- const query: ServerQueryRequest = {
- commandName: partialQuery.commandName,
- args: partialQuery.args === undefined ? [] : partialQuery.args,
- noData: partialQuery.noData === true,
- requestFlags,
- silent: partialQuery.silent === true || requestFlags.benchmark,
- terminateWhenIdle: partialQuery.terminateWhenIdle === true,
- commandFlags: partialQuery.commandFlags === undefined
- ? {}
- : partialQuery.commandFlags,
- cancelToken: partialQuery.cancelToken,
- };
+ const query: ServerQueryRequest = partialServerQueryResponseToFull(
+ partialQuery,
+ );
const {bridge} = client;

ADD
packages/@romejs/core/server/ServerRequest.test.md
+# `ServerRequest.test.ts`
+
+**DO NOT MODIFY**. This file has been autogenerated. Run `rome test packages/@romejs/core/server/ServerRequest.test.ts --update-snapshots` to update.
+
+## `ServerRequest#watchFilesFromArgs extensions`
+
+### `console`
+
+```
+
+```
+
+### `files`
+
+```
+# foo.js
+bar
+
+# foo.txt
+bar
+
+# rome.json
+{
+ "files": {
+ "vendorPath": "../remote"
+ }
+}
+
+```

ADD
packages/@romejs/core/server/ServerRequest.test.ts
+import {test} from "rome";
+import {createIntegrationTest} from "../integrationTestHelpers";
+
+test(
+ "ServerRequest#watchFilesFromArgs extensions",
+ createIntegrationTest(
+ {},
+ async (t, h) => {
+ const req = await h.createRequest();
+ const basenames: Array<string> = [];
+ const sub = await req.watchFilesFromArgs(
+ {
+ extensions: ["txt"],
+ ignoreArgumentMisses: true,
+ },
+ async (paths) => {
+ for (const path of paths) {
+ basenames.push(path.getBasename());
+ }
+ },
+ );
+ await h.writeFile("foo.js", "bar");
+ await h.writeFile("foo.txt", "bar");
+ await sub.unsubscribe();
+ t.inlineSnapshot(basenames, 'Array ["foo.txt"]');
+ },
+ ),
+);

MODIFY
packages/@romejs/core/server/ServerRequest.ts
packages/@romejs/core/server/ServerRequest.ts
@@ -28,10 +28,7 @@ import {
DiagnosticsPrinter,
DiagnosticsPrinterFlags,
} from "@romejs/cli-diagnostics";
-import {
- ProjectConfigCategoriesWithIgnore,
- ProjectDefinition,
-} from "@romejs/project";
+import {ProjectDefinition} from "@romejs/project";
import {ResolverOptions, ResolverQueryResponseFound} from "./fs/Resolver";
import {BundlerConfig} from "../common/types/bundler";
import ServerBridge, {
@@ -126,7 +123,6 @@ export type ServerRequestGetFilesOptions = Omit<
ignoreProjectIgnore?: boolean;
disabledDiagnosticCategory?: DiagnosticCategory;
advice?: DiagnosticAdvice;
- configCategory?: ProjectConfigCategoriesWithIgnore;
verb?: string;
noun?: string;
args?: Array<string>;
@@ -589,10 +585,7 @@ export default class ServerRequest {
async watchFilesFromArgs(
opts: ServerRequestGetFilesOptions,
- callback: (
- result: ServerRequestGetFilesResult,
- initial: boolean,
- ) => Promise<void>,
+ callback: (paths: AbsoluteFilePathSet, initial: boolean) => Promise<void>,
): Promise<EventSubscription> {
this.checkCancelled();
@@ -604,40 +597,30 @@ export default class ServerRequest {
let timeout: undefined | NodeJS.Timeout;
let runningCallback = false;
- let unsubscribed = false;
-
- let pendingResult: ServerRequestGetFilesResult = {
- paths: new AbsoluteFilePathSet(),
- projects: new Set(),
- };
+ let pendingPaths = new AbsoluteFilePathSet();
async function flush(initial: boolean) {
- if (unsubscribed) {
- return;
- }
-
- if (!initial && pendingResult.paths.size === 0) {
+ if (!initial && pendingPaths.size === 0) {
return;
}
timeout = undefined;
- const result = pendingResult;
- pendingResult = {
- paths: new AbsoluteFilePathSet(),
- projects: new Set(),
- };
+ const paths = pendingPaths;
+ pendingPaths = new AbsoluteFilePathSet();
runningCallback = true;
- await callback(result, initial);
+ await callback(paths, initial);
runningCallback = false;
- if (pendingResult.paths.size > 0) {
+ if (pendingPaths.size > 0) {
await flush(false);
}
}
- const onChange = (path: AbsoluteFilePath) => {
+ const refreshFileEvent = this.server.refreshFileEvent.subscribe((
+ path: AbsoluteFilePath,
+ ) => {
let matches = false;
for (const arg of resolvedArgs) {
if (arg.path.equal(path) || path.isRelativeTo(arg.path)) {
@@ -649,42 +632,43 @@ export default class ServerRequest {
return;
}
- const project = this.server.projectManager.findProjectExisting(path);
- if (project !== undefined) {
- pendingResult.projects.add(project);
+ const paths = this.server.memoryFs.glob(path, opts);
+ for (const path of paths) {
+ pendingPaths.add(path);
}
- pendingResult.paths.add(path);
-
// Buffer up evicted paths
if (!runningCallback && timeout === undefined) {
timeout = setTimeout(() => flush(false), 100);
}
- };
+ });
- const refreshFileEvent = this.server.refreshFileEvent.subscribe(onChange);
+ const endSubscription = this.endEvent.subscribe(() => {
+ sub.unsubscribe();
+ });
const sub = mergeEventSubscriptions([
refreshFileEvent,
{
- unsubscribe() {
- unsubscribed = true;
+ async unsubscribe() {
if (timeout !== undefined) {
clearTimeout(timeout);
+
+ // Run the timeout right now
+ await flush(false);
}
},
},
]);
- this.endEvent.subscribe(() => {
- sub.unsubscribe();
- });
-
// Flush initial
- pendingResult = await this.getFilesFromArgs(opts);
+ const pendingResult = await this.getFilesFromArgs(opts);
+ for (const path of pendingResult.paths) {
+ pendingPaths.add(path);
+ }
await flush(true);
- return sub;
+ return mergeEventSubscriptions([endSubscription, sub]);
}
async getFilesFromArgs(
@@ -699,21 +683,13 @@ export default class ServerRequest {
opts.tryAlternateArg,
);
- const extendedGlobOpts: MemoryFSGlobOptions = {...opts};
-
- if (configCategory !== undefined) {
- extendedGlobOpts.getProjectIgnore = (project) =>
- ignoreProjectIgnore ? [] : project.config[configCategory].ignore
- ;
- }
-
// Resolved arguments that resulted in no files
const noArgMatches: Set<ResolvedArg> = new Set();
// Match files
const paths: AbsoluteFilePathSet = new AbsoluteFilePathSet();
for (const arg of resolvedArgs) {
- const matches = server.memoryFs.glob(arg.path, extendedGlobOpts);
+ const matches = server.memoryFs.glob(arg.path, opts);
if (matches.size === 0) {
if (!opts.ignoreArgumentMisses) {

MODIFY
packages/@romejs/core/server/fs/MemoryFileSystem.ts
packages/@romejs/core/server/fs/MemoryFileSystem.ts
@@ -17,6 +17,7 @@ import {
parsePathPattern,
} from "@romejs/path-match";
import {
+ ProjectConfigCategoriesWithIgnore,
ProjectDefinition,
ROME_CONFIG_FILENAMES,
ROME_CONFIG_PACKAGE_JSON_FIELD,
@@ -106,8 +107,8 @@ type DeclareManifestOpts = {
};
type CrawlOptions = {
+ reason: "watch" | "initial";
diagnostics: DiagnosticsProcessor;
- crawl: boolean;
onFoundDirectory?: (path: AbsoluteFilePath) => void;
tick?: (path: AbsoluteFilePath) => void;
};
@@ -125,7 +126,7 @@ export type WatcherClose = () => void;
export type MemoryFSGlobOptions = {
extensions?: Array<string>;
overrideIgnore?: PathPatterns;
- getProjectIgnore?: (project: ProjectDefinition) => PathPatterns;
+ configCategory?: ProjectConfigCategoriesWithIgnore;
test?: (path: AbsoluteFilePath) => boolean;
};
@@ -180,12 +181,12 @@ async function createWatcher(
]);
if (newStats.type === "file") {
- memoryFs.handleFileChange(
+ memoryFs.addFile(
path,
newStats,
{
+ reason: "watch",
diagnostics,
- crawl: true,
},
);
} else if (newStats.type === "directory") {
@@ -193,7 +194,7 @@ async function createWatcher(
path,
newStats,
{
- crawl: true,
+ reason: "watch",
diagnostics,
onFoundDirectory,
},
@@ -219,9 +220,9 @@ async function createWatcher(
projectFolder,
stats,
{
- crawl: true,
diagnostics,
onFoundDirectory,
+ reason: "initial",
},
);
logger.info(
@@ -424,8 +425,12 @@ export default class MemoryFileSystem {
// Wait for any subscribers that might need the file's stats
this.server.logger.info(`[MemoryFileSystem] File deleted:`, path.toMarkup());
+
+ // Only emit these events for files
+ if (folderInfo === undefined) {
await this.deletedFileEvent.call(path);
this.server.refreshFileEvent.send(path);
+ }
// Remove from 'all possible caches
this.files.delete(path);
@@ -452,25 +457,6 @@ export default class MemoryFileSystem {
}
}
- async handleFileChange(
- path: AbsoluteFilePath,
- stats: Stats,
- opts: CrawlOptions,
- ): Promise<boolean> {
- const oldStats: undefined | Stats = this.getFileStats(path);
- const changed = await this.addFile(path, stats, opts);
- if (changed) {
- const newStats: Stats = this.getFileStatsAssert(path);
- this.server.logger.info(
- `[MemoryFileSystem] File change:`,
- path.toMarkup(),
- );
- this.server.refreshFileEvent.send(path);
- this.changedFileEvent.send({path, oldStats, newStats});
- }
- return changed;
- }
-
async waitIfInitializingWatch(
projectFolderPath: AbsoluteFilePath,
): Promise<void> {
@@ -713,7 +699,7 @@ export default class MemoryFileSystem {
cwd: AbsoluteFilePath,
opts: MemoryFSGlobOptions = {},
): AbsoluteFilePathSet {
- const {extensions, getProjectIgnore, test, overrideIgnore = []} = opts;
+ const {extensions, configCategory, test, overrideIgnore = []} = opts;
const paths: AbsoluteFilePathSet = new AbsoluteFilePathSet();
@@ -724,15 +710,22 @@ export default class MemoryFileSystem {
while (crawl.length > 0) {
const path = crawl.pop()!;
- const project = this.server.projectManager.assertProjectExisting(path);
+ // `cache: false` to allow calling us with deleted paths
+ const project = this.server.projectManager.findProjectExisting(
+ path,
+ false,
+ );
let ignore: PathPatterns = overrideIgnore;
// Get ignore patterns
- if (getProjectIgnore !== undefined) {
+ if (configCategory !== undefined && project !== undefined) {
const projectIgnore = ignoresByProject.get(project);
if (projectIgnore === undefined) {
- ignore = concatGlobIgnore([...ignore, ...getProjectIgnore(project)]);
+ ignore = concatGlobIgnore([
+ ...ignore,
+ ...project.config[configCategory].ignore,
+ ]);
ignoresByProject.set(project, ignore);
} else {
ignore = projectIgnore;
@@ -845,7 +838,6 @@ export default class MemoryFileSystem {
opts.onFoundDirectory(folderPath);
}
- if (opts.crawl) {
// Crawl the folder
const files = await readDirectory(folderPath);
@@ -869,7 +861,6 @@ export default class MemoryFileSystem {
// Add the rest of the items
await Promise.all(Array.from(files, declareItem));
- }
return true;
}
@@ -950,6 +941,17 @@ export default class MemoryFileSystem {
});
}
+ // Detect file changes
+ const oldStats = this.getFileStats(path);
+ if (oldStats !== undefined && opts.reason === "watch") {
+ this.server.logger.info(
+ `[MemoryFileSystem] File change:`,
+ path.toMarkup(),
+ );
+ this.server.refreshFileEvent.send(path);
+ this.changedFileEvent.send({path, oldStats, newStats: stats});
+ }
+
return true;
}
}

MODIFY
packages/@romejs/core/server/linter/Linter.ts
packages/@romejs/core/server/linter/Linter.ts
@@ -509,7 +509,7 @@ export default class Linter {
return this.request.watchFilesFromArgs(
this.getFileArgOptions(),
- async ({paths: evictedPaths}, initial) => {
+ async (evictedPaths, initial) => {
const processor = this.createDiagnosticsProcessor(evictedPaths, runner);
const result = await runner.run({firstRun, evictedPaths, processor});
@@ -592,7 +592,7 @@ export default class Linter {
},
});
- watchEvent.unsubscribe();
+ await watchEvent.unsubscribe();
const printer = createDiagnosticsPrinter(
request,

MODIFY
packages/@romejs/core/test-worker/TestWorkerRunner.ts
packages/@romejs/core/test-worker/TestWorkerRunner.ts
@@ -162,7 +162,11 @@ export default class TestWorkerRunner {
prettyFormat(arg, {allowCustom: false, markup: true})
);
}
- const text = textParts.join(" ");
+ let text = textParts.join(" ");
+
+ if (text === "") {
+ text = "<dim>empty log</dim>";
+ }
const err = new Error();

MODIFY
packages/@romejs/diagnostics/derive.ts
packages/@romejs/diagnostics/derive.ts
@@ -113,11 +113,21 @@ export function deriveRootAdviceFromDiagnostic(
});
}
+ const message = description.message.value;
+
+ if (message === "") {
+ advice.push({
+ type: "log",
+ category: "none",
+ text: "<dim>no diagnostic message specified</dim>",
+ });
+ } else {
advice.push({
type: "log",
category: "error",
- text: description.message.value,
+ text: message,
});
+ }
if (opts.skipFrame === false) {
if (location.start !== undefined && location.end !== undefined) {

MODIFY
packages/@romejs/events/Event.ts
packages/@romejs/events/Event.ts
@@ -181,7 +181,7 @@ export default class Event<Param, Ret = void> {
this.onSubscriptionChange();
return {
- unsubscribe: () => {
+ unsubscribe: async () => {
this.unsubscribe(callback);
},
};

MODIFY
packages/@romejs/events/types.ts
packages/@romejs/events/types.ts
@@ -27,7 +27,7 @@ export type EventOptions = {
};
export type EventSubscription = {
- unsubscribe: () => void;
+ unsubscribe: () => Promise<void>;
};
export type BridgeHandshakeMessage = {

MODIFY
packages/@romejs/events/utils.ts
packages/@romejs/events/utils.ts
@@ -11,9 +11,9 @@ export function mergeEventSubscriptions(
subs: Array<EventSubscription>,
): EventSubscription {
return {
- unsubscribe() {
+ async unsubscribe() {
for (const sub of subs) {
- sub.unsubscribe();
+ await sub.unsubscribe();
}
},
};

Filter file changes for watchFilesFromArgs - fixes #629 (#630)