-
Notifications
You must be signed in to change notification settings - Fork 0
Review 5259
andrskr (contributor, fork) · OPEN · view on GitHub
Verdict: approve (merge)
Head reviewed: a091dde49ee59e5e82c44c35aee8bc33c3915f6a
A builder whose project configures an integration types astryx search "otp" and
gets docs and templates but never the integration's own OtpField; astryx build
recommends TextInput instead. The same project's astryx component --list lists
it — so the single "I'm looking for X" entry point silently omits components that
are installed, configured and documented. Repro in #5258.
Search's component pool becomes core's tree plus every configured integration's components, with the owning package as the import hint.
Builders on projects that configure integrations; agents driving --json see
extra rows. Projects with no integrations are untouched — loadIntegrationsSafely
returns [].
No API change — gatherComponents/gatherCoreComponents/gatherIntegrationComponents
are module-private (search.mjs:363, 401, 427); exports and the response envelope
are unchanged.
No new theme targets — CLI path.
- API — no. One meaning change: a project with integrations gets rows it did not before. That is the fix.
- Visual — no, terminal output. Theme — no.
One extra Project.load(cwd) per run (search.mjs:402); gatherDocs already does
one. One dynamic import() per integration doc, overlapped in Promise.all.
Zero-integration projects: one config read, no doc imports.
Low-risk — no new API surface, no behavior regression (11 pre-existing search tests pass), no perf regression class.
approve — merge
1. [not blocking] import hint is the bare package name
→ a builder copies `@acme/widgets` while `astryx component FancyGizmo` prints
`@acme/widgets/Widgets` when the doc's directory is an exported subpath
· search.mjs:413
2. [not blocking, NOT theirs] the fix reaches configured integrations only
→ back-compat `pkg.astryx.docs` externals stay in `component --list` and absent
from search. Pre-existing; ours to close · list/list.mjs:186
3. [not blocking] no changeset
→ the CLI fix lands with no release note and no patch bump
Test evidence: whole file 13/13 at head; with the fix reverted, 2 fail — exactly the two new cases. The test genuinely covers the new path.
Thanks — search and build finally agreeing with
component --listis the right fix, and the test fails without it. Merging as is; one optional nit inline.
-
packages/cli/api/search/search.mjs:413—component <Name>resolves the exports subpath (_adapter.mjs:405) — this hint stays at package level.
Posted as drafted.
CI has never run on this PR — all four workflows sit action_required (first-time
fork contributor). A maintainer must release them before merge means anything.