fix(config): warn instead of erroring when workspace member dir is missing#34511
Conversation
…ssing Listing a workspace member by literal path required the directory to be present on disk, which broke the documented Docker workflow that only copies a subset of workspace members. Match npm's behavior: emit a warning and skip the missing member. Genuine misconfigurations (e.g. an npm workspace member that exists but has no package.json) still error. Fixes denoland#28365
fibibot
left a comment
fibibot
left a comment
There was a problem hiding this comment.
Skip-and-warn for a missing member is correct, and the pkg_json().is_none() branch in the npm loop still errors for a present-but-package.json-less member (test_npm_workspace_member_only_deno_json_errors covers it).
One thing to confirm: find_member_config_folder returns NotFound whenever load_config_folder finds no config file, so this now also skips a member directory that exists but has no deno.json/deno.jsonc, not only a truly-missing one. For a deno workspace that means a typo'd member path or a member you forgot to add a config to is silently dropped with just a warning, where it previously errored. Intended? The PR body's "still errors" guarantee only holds for an npm member that has a deno.json but lacks package.json.
Holding approval until CI is green (26 checks still pending).
- nit: the two warnings disagree in shape — the deno-loop message is
Workspace member "{raw_member}" not found at {dir_url}, the npm-loop one isWorkspace member directory "{dir_url}" not found. Align them; includingraw_memberreads better.
Previously the warn-and-skip path triggered whenever no config file was found in the member dir, which also masked a typo'd member path or a forgotten deno.json/package.json as a silent skip. Now we only skip when the dir does not exist on disk; a present directory missing its config keeps the original error. Also align the warn message in both loops to include the raw member name.
lunadogbot
left a comment
lunadogbot
left a comment
There was a problem hiding this comment.
Commit 194fee1 precisely addresses my prior concern about silent-skip masking typo'd member paths. The skip is now gated on !member_dir_exists(sys, &dir_url), so:
- Missing dir → warn + skip (the Docker scenario from #28365).
- Present dir, no
deno.json/deno.jsonc→ original NotFound still surfaces. - Present dir, no
package.json(npm loop) → upgraded to the more specificNotFoundPackageJson.
Test coverage hits the four corners cleanly: test_deno_workspace_member_missing_dir_skipped, test_deno_workspace_member_dir_without_config_errors, test_npm_workspace_member_missing_dir_skipped, test_npm_workspace_member_dir_without_config_errors, plus test_npm_workspace_member_only_deno_json_errors for the deno.json-present-but-package.json-absent case. The two warning shapes were also aligned to Workspace member "{raw}" not found at {url}. Skipping. — the npm loop collects (raw_member, member_dir_url) tuples so the message has the raw name to print.
Minor non-blocker: for explicit path_members, raw_member is the literal config string (e.g. ./apps/web); for glob-matched pkg_json_paths, raw_member is the absolute dir path. The warning text will read differently between the two sources. Almost certainly invisible in practice for the bug-report case, but storing the relative form on the glob path would be nicer for symmetry.
CI is green except test node_compat (1/3) debug windows-x86_64 still IN_PROGRESS, unrelated to workspace discovery.
LGTM.
…ssing (denoland#34511) Listing a workspace member by literal path required the directory to be present on disk, otherwise workspace discovery failed with "Could not find config file for workspace member". This broke the documented Docker workflow that only copies the relevant member subset into the image, and it diverged from npm, which silently tolerates missing workspace members. Now we emit a warning and skip the missing member, so any remaining members install and run normally. Genuine misconfigurations are unchanged: pointing at a config file instead of a directory still errors, and an npm workspace member whose directory exists but contains no `package.json` still errors. Fixes denoland#28365
3 participants
Listing a workspace member by literal path required the directory to be
present on disk, otherwise workspace discovery failed with "Could not
find config file for workspace member". This broke the documented Docker
workflow that only copies the relevant member subset into the image, and
it diverged from npm, which silently tolerates missing workspace
members. Now we emit a warning and skip the missing member, so any
remaining members install and run normally.
Genuine misconfigurations are unchanged: pointing at a config file
instead of a directory still errors, and an npm workspace member whose
directory exists but contains no
package.jsonstill errors.Fixes #28365