fix(react-router/fs-routes): correctly apply ignoredFilePatterns to files and folders in flatRoutes

[rururux]

fixes: #14157

The current implementation checks ignoredFilePatterns against file paths relative to the app directory (e.g. "routes/mainPage.tsx") instead of the routes directory as documented (e.g. "mainPage.tsx"). As a result, patterns like "mainPage.tsx" do not match and the behavior is surprising for users.
This PR fixes that by using paths relative to the routes directory when evaluating ignoredFilePatterns, restoring the documented behavior.

[changeset-bot]

🦋 Changeset detected

Latest commit: c5e7159

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@react-router/fs-routes	Patch
create-react-router	Patch
react-router	Patch
react-router-dom	Patch
@react-router/architect	Patch
@react-router/cloudflare	Patch
@react-router/dev	Patch
@react-router/express	Patch
@react-router/node	Patch
@react-router/remix-routes-option-adapter	Patch
@react-router/serve	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[rururux]

While working on this issue I discovered another potential problem in a specific case.
For users who define routes by folder (e.g. routes/main/route.tsx) and want to exclude a particular folder (e.g. main), they will typically specify either "main" or "main/*".
The former works fine, but the latter fails. The current implementation compares the folder name (the route name) directly against ignoredFilePatterns, so patterns containing "/*" will not match correctly.
However, "main/*" is a natural pattern for users who want to exclude all files inside that folder, and treating this pattern as unsupported can lead to surprising behavior.
I propose the following fix to address this issue.

function findRouteModuleForFolder(
  appDirectory: string,
  routesDir: string,
  filepath: string,
  ignoredFileRegex: RegExp[],
): string | null {
- let relativePath = path.relative(routesDir, filepath);
+ let relativePath = normalizeSlashes(path.relative(routesDir, filepath));
- let isIgnored = ignoredFileRegex.some((regex) => regex.test(relativePath));
+ let isIgnored = ignoredFileRegex.some((regex) => regex.test(relativePath) || regex.test(relativePath + "/*"));
  if (isIgnored) return null;

[rururux]

I acknowledge this is an incomplete solution, but note that React Router usage disallows nested folders as a constraint.
Also, although I said "exclude all files", in practice only route.[jt]sx or index.[jt]sx files inside the folder are considered when defining routes.
Therefore patterns like "main/route.tsx" won't actually match, but such specifications are rare and I intentionally excluded them.

[rururux]

Hi @brophdawg11,
​I've been working on this PR to fix the core ignoredFilePatterns issue. While doing so, I also found another potential problem related to main/* patterns in the folder-based routing.

​My proposal to fix that is outlined in the comment section. I've left the PR in draft status to get your feedback on this proposed approach first.

​Could you please take a look and let me know if this is the right direction for the project? If so, I'll implement the fix and mark the PR as ready for review.
​Thanks for your time!

[markdalgleish]

Thanks for the PR!

I think you're right that the current behaviour is not ideal and your proposed behaviour of matching relative to the routes directory is much better. However, this would be a breaking change for anyone already using this feature.

We could potentially add a flag to change this behaviour, or deprecate the existing ignoredRouteFiles option and introduce a new one, but I'd rather not expand (and arguably muddy) the API just to handle something that users can already do. Especially since, to me, route directories solve the original problem more cleanly anyway. If a route needs additional files (CSS, etc.) then the route should become a directory, rather than ignoring routes/*.css files.

I think the main problem to be fixed is that the documentation is both wrong and incomplete. We should be documenting that patterns are relative to the app directory, and giving better examples of patterns that can be used (potentially linking out to minimatch since that's what we use internally). For example, to your point about directories, you can ignore them like this: **/directory-name/**/*.

If you'd like, you could update this PR with the following changes and I'd be happy to merge it:

• Roll back the behaviour changes in flatRoutes.ts.
• Keep the tests you've added, but update them to test the existing behaviour.
• Update all references to ignoredRouteFiles in the docs so they describe the behaviour correctly.
• Ensure the PR is pointed at the main branch rather than dev now that it's mostly just docs changes that we'd like to go out immediately.

[rururux]

Sorry for the late reply, and thanks for your response!
I understand that it would be a breaking change and that it would be better to change the documentation instead.
I will make the changes as you suggested.

[rururux]

I re-examined the issue, but there is one problem.
In the current implementation, when a folder is detected in the routes folder, such as routes/home/route.tsx, the system verifies whether it matches the ignoredFilePatterns using the relative path to the folder itself, rather than the routing file within the folder.

react-router/packages/react-router-fs-routes/flatRoutes.ts

Lines 310 to 338 in 2df250e

	function findRouteModuleForFolder(
	appDirectory: string,
	filepath: string,
	ignoredFileRegex: RegExp[],
	): string | null {
	let relativePath = path.relative(appDirectory, filepath);
	let isIgnored = ignoredFileRegex.some((regex) => regex.test(relativePath));
	if (isIgnored) return null;
	let routeRouteModule = findFile(filepath, "route", routeModuleExts);
	let routeIndexModule = findFile(filepath, "index", routeModuleExts);
	// if both a route and index module exist, throw a conflict error
	// preferring the route module over the index module
	if (routeRouteModule && routeIndexModule) {
	let [segments, raw] = getRouteSegments(
	path.relative(appDirectory, filepath),
	);
	let routePath = createRoutePath(segments, raw, false);
	console.error(
	getRoutePathConflictErrorMessage(routePath || "/", [
	routeRouteModule,
	routeIndexModule,
	]),
	);
	}
	return routeRouteModule || routeIndexModule || null;
	}

In other words, even if there is a file like routes/home/route.tsx, the string used to verify whether it matches the ignoredFilePatterns is routes/home. There is no process to recheck whether the file within the home folder matches the ignoredFilePatterns.
Therefore, in the current implementation, if you want to match routes/home.tsx, you must write routes/home.tsx, and if you want to match routes/home/route.tsx, you must also write routes/home.tsx, which is a bit cumbersome.

For example, this test will always fail.

test("should ignore files in a folder", () => {
  let routeOne = path.join(routesDir, "one.tsx");
  let routeTwoDir = path.join(routesDir, "two");
  let routeTwo = path.join(routeTwoDir, "route.tsx");
  writeFileSync(routeOne, "");
  mkdirSync(routeTwoDir, { recursive: true });
  writeFileSync(routeTwo, "");

  // OK
  // let ignoredFilePatterns = ["routes/two"];
  // FAIL
  let ignoredFilePatterns = ["routes/two/*.tsx"];
  let routeManifest = flatRoutes(tempDir, ignoredFilePatterns);

  let routeIds = Object.keys(routeManifest);

  expect(routeIds).not.toContain("routes/two");
  expect(routeIds.length).toBe(1);
});