cleanup(core): move esbuild to use fdir/picomatch #28037
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 0b18ded. 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
Sent with 💌 from NxCloud. |
|
No worries, I'll have a look soon 👍 Will catch the branch up too |
43081j
force-pushed
the
esbuild-fdir
branch
from
fe5966f to
1080c1d
Compare
43081j
force-pushed
the
esbuild-fdir
branch
from
1080c1d to
ca8ff73
Compare
|
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 |
43081j
force-pushed
the
esbuild-fdir
branch
from
ca8ff73 to
c59a4d7
Compare
43081j
force-pushed
the
esbuild-fdir
branch
from
c59a4d7 to
726d35e
Compare
43081j
force-pushed
the
esbuild-fdir
branch
from
726d35e to
20a560a
Compare
|
@43081j I pulled in latest again after all the recent merges just in case |
|
I think there might be some flake in the e2e results, but the |
|
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 |
Migrates away from fast-glob to `fdir` and `picomatch`, a faster and smaller combination.
|
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 |
|
Oh wow, whatever got merged in from master seems to have fixed the tests! Any chance I can get a re-review? 🙏 |
|
@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: micromatch/picomatch#136 |
|
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 👍 |
|
You might consider
I'm not sure that issue is going to have an effect. |
|
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 |
An alternative to nrwl#28037, switches the esbuild package to use `tinyglobby` instead of `fast-glob`. This improves performance a fair chunk and reduces install complexity/size.
An alternative to nrwl#28037, switches the esbuild package to use `tinyglobby` instead of `fast-glob`. This improves performance a fair chunk and reduces install complexity/size.
|
Closing because we are going ahead with #29453 instead |
|
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. |
Migrates away from fast-glob to
fdirandpicomatch.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