Skip to content

Conversation

@outslept
Copy link
Contributor

@outslept outslept commented Aug 20, 2025

resolves #117

continue from #118 (by @aa900031 ❤️)

based on the work from the previous contributor (preserving commit history). this update resolves merge conflicts, brings the implementation up to current state, and hopefully gets it ready for merge (keeping as draft to avoid getting distracted and jumping to other tasks lol).

this is mostly done. keeping this draft to shape the api, etc. I'll add/shape jsdocs/readme when this is good to go

p.s. regarding the ci, autofix action seem to override lint:fix on his own. so in my case it was with the rule unicorn/no-nested-ternary. I was adding it locally -> autofix ci job passes -> it removes those brackets causing a eslint problem we were trying to prevent

@outslept outslept force-pushed the feat/resolve-workspace branch from 35797c8 to 4059a87 Compare August 20, 2025 03:25
autofix-ci bot and others added 2 commits August 20, 2025 03:26
@pi0 pi0 changed the title workspace utils feat: workspace utils Aug 20, 2025
@pi0
Copy link
Member

pi0 commented Aug 20, 2025

Thanks for resolving conflicts from the original PR.

It might still take me longer to properly review, as I really want to make sure implementation is as minimal as possible.

As an alternative direction we could publish it as a standalone package and probably if adopted host under unjs :)

@outslept
Copy link
Contributor Author

outslept commented Aug 20, 2025

Sure, take your time - no rush at all! If we keep it minimal - I think keeping it here makes sense since it's mostly closely related to package.json handling and workspace detection that pkg-types already does to an extent

@pi0
Copy link
Member

pi0 commented Aug 20, 2025

Yeah, basic support makes sense. I'm also a little bit worried about the install size, too, though pkg-types is currently ~280kB and with tiny-globby ~158kB + bundled logic from PR (have to measure) we will be adding bytes that might not be used by big percentage of pkg-types users (or might be! that's also what we have to think more about)

@outslept
Copy link
Contributor Author

outslept commented Aug 20, 2025

Unfortunately there's not really much to do. There's a tiny-glob package, which is really small (the smallest of them all afaik), but because of this is, it is sort of not reliable for production use, so currently tinyglobby provides the best of both world. Also fs.glob() may be an option as pkg-types does not set engines. But it's only in the future sadly..

@pi0
Copy link
Member

pi0 commented Aug 20, 2025

Since glob is only needed for resolveWorkspacePackages (and not resolveWorkspace) we could require providing a glob implementation to function could also default to node.glob with notice about 20x compat (requring polyfill)

@outslept
Copy link
Contributor Author

outslept commented Aug 20, 2025

Sounds good. The only correction is that node.glob is 22x and above 😄

@pi0
Copy link
Member

pi0 commented Aug 20, 2025

We could also implement an internal glob method as alternative. pathe (already a dep) supports matchesGlob to be used as basis.

@outslept
Copy link
Contributor Author

This can work as well. But we need a crawler in that case. tinyglobby uses fdir, perhaps you've made one already inside unjs?

@pi0 pi0 changed the title feat: workspace utils feat: resolveWorkspace, resolveWorkspacePackages and resolveWorkspaceGraph Aug 20, 2025
@pi0 pi0 mentioned this pull request Aug 20, 2025
@pi0
Copy link
Member

pi0 commented Aug 20, 2025

fdir is good but it costs also another 50Kb 😆 Let's just simplify story and use native fs.glob + allow bringing custom glob to function and let users choose impl.

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.

workspace (monorepo) utils

5 participants