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

Replace glob packages with lighter alternatives – fast-glob + is-glob -> tinyglobby, micromatch -> picomatch #3680

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

Conversation

pralkarz
Copy link

@pralkarz pralkarz commented Mar 8, 2025

Related to the lately ongoing e18e efforts (e18e/ecosystem-issues#164). package-lock doesn't change much, but the heavier dependencies become dev-only.

@pralkarz pralkarz requested a review from zachleat as a code owner March 8, 2025 12:52
Comment on lines -196 to -234
test("Bad expected output, this indicates a bug upstream in a dependency (update, was fixed in [email protected]). Input to 'src' and empty includes dir (issue #403, full paths in eleventyignore)", async (t) => {
let eleventyConfig = await getTemplateConfigInstanceCustomCallback({
input: "test/stubs-403",
output: "_site",
includes: "",
data: false,
}, function(eleventyConfig) {
eleventyConfig.setUseGitIgnore(false);
});

let { eleventyFiles: evf } = getEleventyFilesInstance(["liquid"], eleventyConfig);
evf._setEleventyIgnoreContent("!" + TemplatePath.absolutePath("test/stubs-403/_includes") + "/**");
evf.init(); // duplicate init

t.deepEqual(await evf.getFiles(), [
"./test/stubs-403/template.liquid",
// UPDATE: this was fixed in [email protected]
// This should be excluded from this list but is not because the ignore content used an absolutePath above.
// "./test/stubs-403/_includes/include.liquid",
]);
});

test("Workaround for Bad expected output, this indicates a bug upstream in a dependency. Input to 'src' and empty includes dir (issue #403, full paths in eleventyignore)", async (t) => {
let eleventyConfig = await getTemplateConfigInstanceCustomCallback({
input: "test/stubs-403",
output: "_site",
includes: "",
data: false,
}, function(eleventyConfig) {
eleventyConfig.setUseGitIgnore(false);
});

let { eleventyFiles: evf } = getEleventyFilesInstance(["liquid"], eleventyConfig);
evf._setEleventyIgnoreContent("!./test/stubs-403/_includes/**");
evf.init(); // duplicate init

t.deepEqual(await evf.getFiles(), ["./test/stubs-403/template.liquid"]);
});

Copy link
Author

Choose a reason for hiding this comment

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

I removed these tests as from what I gather they were related to a bug in an old fast-glob version. Down to add them back as a sanity check though.

t.deepEqual(evf.getRawFiles(), ["./test/stubs/writeTest/**/*.{liquid,md}"]);
t.true(files.length > 0);
t.is(files[0], "./test/stubs/writeTest/test.md");
t.is(files[0], "test/stubs/writeTest/test.md");
Copy link
Author

@pralkarz pralkarz Mar 8, 2025

Choose a reason for hiding this comment

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

In production, the leading ./s always get added, this is a tests-only change. I can also mimic the expected behavior by mapping on line 135 if you'd prefer that rather than changing the assertion. Same goes for other changes like this.

Comment on lines +260 to +261
// Swapped to `micromatch` in 3.0.0-alpha.17, and later to `picomatch`.
// The test can stay as a sanity check.
Copy link
Author

Choose a reason for hiding this comment

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

I wasn't sure what to do with this comment, so suggestions welcome! 😄

Comment on lines +103 to 104
// `fast-glob` isn't used any more, but the test can stay as a sanity check.
test("fastglob assumptions", async (t) => {
Copy link
Author

Choose a reason for hiding this comment

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

Please let me know whether you'd rather remove this entirely.

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