feat(npm): add support for glob pattern in npm workspaces and pnpm-workspaces.yaml#463
feat(npm): add support for glob pattern in npm workspaces and pnpm-workspaces.yaml#463stevearc merged 7 commits intostevearc:masterfrom
Conversation
| ---@param filepath string | ||
| ---@return table|nil | ||
| ---Parse pnpm-workspace.yaml and extract packages array, returning both inclusions and exclusions | ||
| local function parse_pnpm_workspace(filepath) |
There was a problem hiding this comment.
This function is pretty jank. Could we use treesitter's yaml parser instead, if it's installed? It still wouldn't cover the full yaml spec, but it would have fewer edge cases
There was a problem hiding this comment.
Good point, I'll put the regex one as a fallback then if the yaml treesitter not installed. wdyt?
lua/overseer/template/npm.lua
Outdated
| for mgr, lockfiles in pairs(mgr_lockfiles) do | ||
| for _, lockfile in ipairs(lockfiles) do | ||
| if vim.uv.fs_stat(vim.fs.joinpath(package_dir, lockfile)) then | ||
| return { package = package_file, manager = mgr } | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Could we pull this out into a helper function to reduce the nesting?
lua/overseer/template/npm.lua
Outdated
| end | ||
| end | ||
|
|
||
| if #inclusions > 0 or #exclusions > 0 then |
There was a problem hiding this comment.
We only care about #inclusions > 0, right? Also, we don't seem to use the exclusions anywhere. Is that intentional?
There was a problem hiding this comment.
Somewhat intentional, since pnpm is the only workspace manager that supports exclusion with !.
But we’re not using that part, so I’ll just return inclusions instead, which is all we need.
lua/overseer/template/npm.lua
Outdated
| local workspace_data = files.load_json_file(workspace_package_file) | ||
| if workspace_data and workspace_data.scripts then | ||
| for k in pairs(workspace_data.scripts) do | ||
| table.insert(ret, { | ||
| name = string.format("%s[workspace] %s (%s)", bin, k, workspace_data.name), | ||
| builder = function() | ||
| return { | ||
| cmd = { bin, "run", k }, | ||
| cwd = workspace_path, | ||
| } | ||
| end, | ||
| }) | ||
| end |
There was a problem hiding this comment.
This seems like it could be factored out into a function. It has a lot of similarities with the workspace loading in the previous block of code.
|
Thanks for the PR! |
|
@anasalqoyyum the workspace logic caused some problems (see #486 and #485). I had to revert this behavior for now until we have a better solution. I am not inclined to add a ton of logic to overseer to duplicate the exact workspace pattern search behavior in pnpm. I think a proper solution would be if pnpm exposes some command that allows us to enumerate and parse the workspaces from the root dir. Not sure if this exists yet. |
|
@stevearc I concur, reverting for now makes sense. Ideally the package manager itself (or pnpm) should be the source of truth here, but I don’t think any command exists yet to return the workspaces. I’ll revisit when I find a clean, lperformant way to do it. |
2 participants
This PR adds support for glob pattern in npm/bun/yarn workspaces and also pnpm workspaces.
This also fixes
pick_package_managerbehavior in monorepo scenario, currently it always falls back tonpmif it's insidepackages.Tested using:
Changes
package.jsonor not.Screenshot