cleanup(core): move esbuild to use fdir/picomatch

[43081j]

Migrates away from fast-glob to fdir and picomatch.

this'll improve performance a fair amount (fdir is much faster than fast-glob) and should reduce the install size (picomatch and fdir are both very small).

fast-glob uses micromatch, while this uses picomatch. the difference seems to be that more advanced bash brace expansion isn't supported in picomatch. if that's important to you, its possible we could use micromatch or zeptomatch

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nx-dev	✅ Ready (Inspect)	Visit Preview	Oct 19, 2024 6:01pm

[nx-cloud[bot]]

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0b18ded33468f2c305764e67e477b04893d0c46d. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

---

✅ Successfully ran 4 targets

• nx affected --targets=lint,test,build,e2e,e2e-ci --base=19e765f89a83f7bf0299116d4b424fc707161787 --head=0b18ded33468f2c305764e67e477b04893d0c46d --parallel=3
• nx run-many -t check-imports check-commit check-lock-files check-codeowners documentation --parallel=1 --no-dte
• nx-cloud record -- nx format:check --base=19e765f89a83f7bf0299116d4b424fc707161787 --head=0b18ded33468f2c305764e67e477b04893d0c46d
• nx documentation --no-dte

---

Sent with 💌 from NxCloud.

[43081j]

No worries, I'll have a look soon 👍

Will catch the branch up too

[JamesHenry]

Please ignore the react rspack module federation e2e error, it will be resolved when you sync with latest master. The others might be legit though

[JamesHenry]

@43081j I pulled in latest again after all the recent merges just in case

[JamesHenry]

I think there might be some flake in the e2e results, but the should support non-bundle builds e2e failure for esbuild might be worth trying to reproduce locally

[43081j]

That same test seems to fail locally for me too

Haven't managed to figure out why yet though. It seems to expect a file to exist which doesn't. That suggests the way we compute the input glob is broken or changed but I'm not sure why/how yet

[43081j]

I fixed the failure but now it seems something to do with the angular e2e is failing. I can't see an error in the logs so I'm not sure what's going on

Some install command fails it seems but there's no error as far as I can see

[43081j]

Oh wow, whatever got merged in from master seems to have fixed the tests!

Any chance I can get a re-review? 🙏

[JamesHenry]

@43081j Yeah there was a peer dep issue with the Angular stack that was fixed so I was hoping that would be it. Thanks a lot for your patience in getting this green!

With that said, this is a more significant change than previous ones under this umbrella, so will need some more eyes.

I am also curious about seeing a resolution to this issue I saw when investigating picomatch: https://github.com/micromatch/picomatch/issues/136

[43081j]

I agree

Other projects we have migrated to picomatch didn't make use of the extra edge cases minimatch supported, so they were fairly easy to move. We also moved a few to zeptomatch which is even smaller and often faster than picomatch, but possibly missing some rarely used pattern types.

Anyhow, let me know when we have more reviews, if we want to change something 👍

[benmccann]

You might consider tinyglobby, which uses fdir/picomatch, but could be better performing as it does some optimizations to avoid having fdir crawl folders that are under the ignore pattern. It would also make it so that there are minimal code changes here needing review as tinyglobby and fast-glob are nearly API compatible. Plus tinyglobby is already a dependency via @nx/js.

I am also curious about seeing a resolution to this issue I saw when investigating picomatch: https://github.com/micromatch/picomatch/issues/136

I'm not sure that issue is going to have an effect. fast-glob uses picomatch and doesn't use minimatch, so I don't think anything will change there

[43081j]

If you want to open a branch which uses tinyglobby instead, I'd be happy to close this

Especially since we already have it in another package of the monorepo. Probably makes sense to use the same one

[43081j]

Closing because we are going ahead with #29453 instead

[github-actions[bot]]

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.