TSX is not ignoring symlinked dependencies in monorepo

[DesignByOnyx]

Bug description

I discovered this while working in a monorepo. I am using TSX to watch my application project (/apps/my-app). I was making edits to one of my local dependencies (/packages/utils) and my application was restarting when I wasn't expecting. As I started to investigate I noticed that:

• TSX is not ignoring node_modules and other dependencies
• TSX does not seem to respect --ignore flags
• TSX is watching files located outside of the cwd

All of the above behaviors seem unexpected, especially the last one as it pertains to a monorepo. Here's a repro:

Reproduction

https://github.com/DesignByOnyx/tsx-restart-ignore-bug

Environment

System:
    OS: Linux 5.15 Ubuntu 20.04.3 LTS (Focal Fossa)
    CPU: (16) x64 11th Gen Intel(R) Core(TM) i7-11800H @ 2.30GHz
    Memory: 11.03 GB / 15.48 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 16.19.0 - ~/.nvm/versions/node/v16.19.0/bin/node
    npm: 9.6.4 - ~/.nvm/versions/node/v16.19.0/bin/npm
  npmPackages:
    tsx: ^3.12.6 => 3.12.6

Can you work on a fix?

• I’m interested in opening a pull request to address this issue.

[DesignByOnyx]

I am definitely interested in helping fix this because the switch to nodemon/ts-node/swc was less than pleasant. I currently have a clone of TSX and am successfully building and testing the referenced project above. Here's what I can tell you so far:

The problem code is in here. I have verified the following:

• All imported node_modules are being added to the watch task, effectively bypassing the ignored options passed to chokidar.
• Local dependencies have been resolved to their realpath, meaning local dependencies do not have node_modules in their path.
  • Adding --ignore **/packages/** does not work - basically nothing is being ignored.

The easiest solution is to check each file against the same ignored options passed to chokidar. I'll take a minimal stab at this to get it working.

[DesignByOnyx]

Here's a fix which does what is expected. My local dependencies (monorepo "packages") are still watched by default. I don't mind manually ignoring them as this only affects monorepos and there are many situations where you would actually want to watch the local dependencies by default (even though they are outside the cwd).

#227

[privatenumber]

I added a test in 412a9c4 to verify changes in dependencies are being ignored.

"TSX not ignoring node_modules or respecting --ignore flags" seems inaccurate. Seems you're specifically talking about monorepos?

[DesignByOnyx]

Hey, so I did some more testing and I found that it has nothing to do with a monorepo per se, but rather if the CWD of the watch script is on a nested directory... which happens a lot in a monorepo.

I greatly simplified the sample project to remove workspaces and to show to reproduce the bug with very little fanfare. I completely removed the "workspaces" entry and simplified the code. The README should contain everything you need.

https://github.com/DesignByOnyx/tsx-restart-ignore-bug

[privatenumber]

@mikemklee @RDeluxe

The issue is about tsx not ignoring dependencies (not about how to include them in the watcher).

Issues are reserved for resolving the root issue. Please avoid off topic conversation.

[kingston]

I was researching this in more detail since I work in a mono-repo and TSX is consuming a substantial amount of available file handles on my Mac. I use PNPM so many dependencies are found in the root directory instead of the individual package so TSX is listening for changes across node_modules and is often listening to 7000+ files.

Root Cause

I deep-dived the code across TSX and Chokidar and I think I found the root cause.

In src/watch/index.ts, the default ignore list is:

// Hidden directories like .git
'**/.*/**',

// Hidden files (e.g. logs or temp files)
'**/.*',

// 3rd party packages
'**/{node_modules,bower_components,vendor}/**',

In version 3.6.0 of chokidar, they normalize all ignore paths as follows (link):

  return normalizePathToUnix(sysPath.isAbsolute(path) ? path : sysPath.join(cwd, path));

Since **/.../** is not an absolute path, it gets joined with the cwd. Therefore, all the ignore paths are only applicable to sub-directories of the current working directory. I confirmed this by modifying chokidar to log the normalized ignore paths during the tests and found it converted to '/tmp/fs-fixture-1726823498420-46/process-directory/**/{node_modules,bower_components,vendor}/**'

Possible Solutions

Therefore as I see it, there are several solutions:

• Make the ignore paths absolute (easiest): Adding "/" in front of each of the ignore paths forces chokidar to recognize it is absolute and avoids joining with cwd. Pros: 4 character fix, Cons: Means anyone using **/.../** in exclude pattern will not get the right behavior
• Switch to custom ignore logic: Chokidar v4 removes glob support so we'd be forced to write our own ignore logic going forward. Add globby/minimatch/whatever else and check each path manually.
• Use a different library: I saw there was mention of using @parcel/watcher which I haven't looked at so maybe they have a better ignore logic.

Given there are several solutions, I held off writing a PR but would be curious to get your thoughts.

Workaround

This appears to be a workaround in the meantime for ignoring all node_modules which stopped TSX from listening to parent node_modules:

tsx watch --ignore /**/node_modules/** ./src

Automated Test

Bonus automated test that fails with the current repo adapted from the test in the "exclude (ignore)" section (tests/specs/watch.ts):

test('with parent directory', async ({ onTestFail }) => {
const entryFile = 'process-directory/index.js';
const fileA = 'file-a.js';
const fileB = 'directory/file-b.js';
const depA = 'node_modules/a/index.js';

await using fixtureGlob = await createFixture({
	[fileA]: 'export default "logA"',
	[fileB]: 'export default "logB"',
	[depA]: 'export default "logC"',
	[entryFile]: `
		import valueA from '../${fileA}'
		import valueB from '../${fileB}'
		import valueC from '../${depA}'
		console.log(valueA, valueB, valueC)
	`.trim(),
});

const tsxProcess = tsx(
	[
		'watch',
		'--clear-screen=false',
		`--ignore=../${fileA}`,
		`--ignore=abra.js`,
		'--exclude=../directory/*',
		'index.js',
	],
	join(fixtureGlob.path, 'process-directory'),
);

onTestFail(async () => {
	// If timed out, force kill process
	if (tsxProcess.exitCode === null) {
		console.log('Force killing hanging process\n\n');
		tsxProcess.kill();
		console.log({
			tsxProcess: await tsxProcess,
		});
	}
});

const negativeSignal = 'fail';

await processInteract(
	tsxProcess.stdout!,
	[
		async (data) => {
			if (data.includes(negativeSignal)) {
				throw new Error('should not log ignored file');
			}

			if (data === 'logA logB logC\n') {
				// These changes should not trigger a re-run
				await Promise.all([
					fixtureGlob.writeFile(fileA, `export default "${negativeSignal}"`),
					fixtureGlob.writeFile(fileB, `export default "${negativeSignal}"`),
					fixtureGlob.writeFile(depA, `export default "${negativeSignal}"`),
				]);

				await setTimeout(1000);
				fixtureGlob.writeFile(entryFile, 'console.log("TERMINATE")');
				return true;
			}
		},
		data => data === 'TERMINATE\n',
	],
	9000,
);

tsxProcess.kill();

const p = await tsxProcess;
expect(p.all).not.toMatch('fail');
expect(p.stderr).toBe('');
}, 10_000);

[privatenumber]

@kingston

Nice work! Would you be willing to contribute a fix?

[kingston]

@kingston

Nice work! Would you be willing to contribute a fix?

I could! But would just be curious to know which of the 3 potential solutions would be better

[privatenumber]

So because tsx only watches imported files, there's a strict list of files to watch rather than entire directories.

My understanding is that there is limited benefit to migrating to Parcel Watcher because it's mainly more performant for watching directories. Since we're only watching individual file paths, I was actually thinking of migrating to Node's native FS watch since it uses the same underlying system watcher.

For globbing support, I'm fine with importing a glob library that tests a string against a glob (e.g. glob-to-regexp).

If you anticipate a breaking change, we'll queue it for v5.

[kingston]

@privatenumber yeah, I did some digging. It seems Vite had a similar thought a few years ago (vitejs/vite#12495 (comment)) and they decided not to go in that direction. I looked into the API and I'd also agree that it's probably a bit too low-level, e.g.

On Linux and macOS systems, fs.watch() resolves the path to an inode and watches the inode. If the watched path is deleted and recreated, it is assigned a new inode. The watch will emit an event for the delete but will continue watching the original inode. Events for the new inode will not be emitted. This is expected behavior.

• https://nodejs.org/docs/latest-v18.x/api/fs.html#inodes

So the way I'd see it, we'd have 2 options:

1. Stick with Chokidar (probably upgrade to v5 and use a glob library like micromatch (the one that @parcel/watcher uses)
2. Use @parcel/watcher. Seems like it's a bit more performant (e.g. being able to collate events from large file changes like git checkout in C++ land) but it requires a native library to be installed and their documentation seems a bit lacking as far as I can tell.

I can make a PR for option 1) if that sounds good. (chokidar v5 + micromatch).

I don't think there should be any breaking changes since it's technically fixing the incorrect watch behavior to the intended behavior. However, it could technically be considered a breaking change since we would be no longer tracking changes of node_modules in monorepos which does make it slightly trickier ironically for my use-case since changes to shared code in other packages in the monorepo no longer trigger a reload when changed.

[privatenumber]

Chokidar v4* + micromatch sounds great! Thank you

RE: inodes
Doesn't that mean we just have to re-watch the path every time we detect an unlink/deletion event? Sounds pretty trivial but I might be missing more trade offs.

RE:

However, it could technically be considered a breaking change since we would be no longer tracking changes of node_modules in monorepos

For the final state, I'm hoping we can evaluate the real path of the file and check if that's in node_modules. If it is, ignore it as a dependency. If it's not, watch it as it's a monorepo symlinked package.

[kingston]

@privatenumber alright I've pulled together a draft PR, however I stumbled upon an issue while I was at it which I'll mention in the PR (#656)

Doesn't that mean we just have to re-watch the path every time we detect an unlink/deletion event? Sounds pretty trivial but I might be missing more trade offs.

Sure - that's all in theory but looking at Chokidar there's a lot of extra logic laying around that esp. with cross-platform that could turn into a can of worms quickly. Not sure if it's worth the trade-off especially since Chokidar v4 is quite a bit slimmer.

For the final state, I'm hoping we can evaluate the real path of the file and check if that's in node_modules. If it is, ignore it as a dependency. If it's not, watch it as it's a monorepo symlinked package.

Yeah - that's definitely possible via fs.realpath. I don't know if there are significant implications but I wonder if it's possible to resolve the path before adding it to the watcher since realpath is an asynchronous operation so wouldn't fit in as an "ignored" synchronous function.