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

fix: inconsistent filepath separators #3632

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: inconsistent filepath separators #3632

wants to merge 2 commits into from

Conversation

seleb
Copy link

@seleb seleb commented Jan 23, 2025

Hey there! Ran into the issue described in #3631 where filepaths are being formatted inconsistently on a windows dev machine, and this seems to prevent the dev server from updating to show new content.

Sample result from _globSearch when the server is started with test1.md, and then a new file test2.md is added:

[
  './content/blog/test1.md',
  './content\\blog\\test2.md',
]

The inconsistent format in the newly added file seems to be carried through all the way into the templates without ever being normalized.

This is a bit of a naive fix, but not sure if there's a better place upstream to place it. I'm not familiar with the specifics but it looks like historically there was transformation between templateGlobs and normalizedTemplateGlobs which may be related?

It's also possible this is a bug in fast-glob, which appears to be responsible for performing the actual search, but as this causes issues with the 11ty dev server I figured it'd be worth raising a PR where there's a known user-facing issue/possible fix and let the maintainers make that call.

@seleb seleb requested a review from zachleat as a code owner January 23, 2025 22:47
ignore: this.uniqueIgnores,
});
})).map(i => i.replace(/\\/g, '/'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Using path.sep would work here, too?

Instead of an IIFE for the promise I think it's okay to add another temporary variable.

Suggested change
})).map(i => i.replace(/\\/g, '/'));
const results = await this.fileSystemSearch.search("templates", globs, {
ignore: this.uniqueIgnores,
});
return results.map(i => i.replace(new RegExp(path.sep, 'g'), '/'));

Untested, as I'm on Debian Linux.

Copy link
Author

Choose a reason for hiding this comment

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

I'll add the temporary variable for clarity, but path.sep would not work here without being escaped. I don't think there'd be any benefit in using it either: replacing linux path separators would not do anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants