Regenerate Admin component definitions and docs - #4689
Conversation
The components.d.ts aggregate had been generated for a subset of components and then patched by hand, so it had drifted from the components as shipped: AppNav, EmptyState, Number, PressButton and ScrollBox were absent entirely, and internal generator scaffolding (AddedContext, BaseClass, ContextRequestEvent) had leaked in where the shared PolarisCustomElement base class belongs. Regenerated for the full component set, then rebuilt the 2026-10-rc docs payload. Definitions move from Polaris 2.23.0 to 2.26.0: shared.d.ts picks up ProgressProps and generalises the field documentation inherited by Checkbox, Choice, ChoiceList, Option and Switch, and Section documents an Icon in its graphic slot and Tooltip among its accessories. yarn lint and yarn type-check pass.
763912e to
e54c7cc
Compare
kyledurand
left a comment
There was a problem hiding this comment.
Looks like nothing changed except for some wording and the addition of progress component 👍
|
Heads up before this merges — the three The cause: every Admin component's In the base file these didn't exist (Avatar had 4 real members: The Technical detail for a fix (AI-friendly)Symptom
Root cause
Fix direction
How to verify the corrected payload
posted by an AI agent on Jason's behalf |
jas7457
left a comment
There was a problem hiding this comment.
Requesting changes — the regenerated generated_docs_data_v2.json payloads have an unintended regression: each Admin component's members list now includes the full native DOM surface (HTMLElement/Element/Node) as public members (~320 bogus members × 66 components), ballooning each file from ~1.48 MB to ~10.5 MB. This looks like base-class DOM-flatten guard drift during regeneration, not intended output. Details and a fix/verification path in my inline comment: #4689 (comment)
The .d.ts changes and the intended additions (Number, EmptyState, AppNav, Progress, wording, Section Icon/Tooltip) look good — just the docs JSON needs a clean regen.
posted by an AI agent on Jason's behalf
The admin docs generator neutralizes the web-component base class before
running generate-docs so the entire HTMLElement/Element/Node DOM surface
is not flattened into every component's documented members. The guard
only rewrote the `typeof globalThis.HTMLElement` form; the definitions
build now emits the base as an object literal
(`declare const BaseClass: { new (): HTMLElement; prototype: HTMLElement }`),
which the guard missed, silently leaking ~320 public DOM members into
every component and ballooning the payload ~7x.
- Broaden the neutralization to also collapse the object-literal base
form to `any` (name- and whitespace-agnostic).
- Add assertNoDomSurfaceLeak: after generation, fail the build loudly if
any public admin component member is a native DOM member. A future
base-class form change now errors instead of silently shipping a bloated
payload.
Re-ran `yarn docs:admin 2026-10-rc` with the hardened generator. The three generated_docs_data_v2.json payloads no longer carry the flattened HTMLElement/Element/Node member surface on every component, so each drops from ~10.5 MB back to ~1.7 MB. The intended additions (Number, EmptyState, AppNav, Progress) and field-wording changes are retained.
jas7457
left a comment
There was a problem hiding this comment.
This is looking better. Having Kyle also run an agent review to make sure things are good now before merging.
kyledurand
left a comment
There was a problem hiding this comment.
from Kyle's agent review
The native-DOM payload regression is fixed at this head. I reproduced the checked-in docs byte-for-byte (1,683,007 bytes per copy); omitting neutralization produced about 10.52 MB and the new assertion correctly rejected all 65 public component classes. No runtime-JS bundle increase is introduced here. Exported per-component declarations grow only 801,198 → 803,029 bytes; the larger increases are in the docs-input aggregate and docs JSON.
I've left inline comments for the remaining docs correction, changeset wording, failure cleanup, and measured opportunities to reduce metadata weight. Generator issues that already affect exported types are explicitly marked as pre-existing follow-ups, not new runtime blockers for this PR. Generated declaration issues should be fixed in the producer and regenerated, rather than patched by hand here.
Additional follow-ups outside the changed lines:
- Shared aliases:
components/Number.d.ts:11importsTextProps, butshared.d.tsdeclaresTextProps$1. With real dependencies and TypeScript 5.9.3,NumberProps['tone']resolves toanyand accepts an invalid tone. This predates this PR; by contrast, this regeneration fixesProgressProps['max']to resolve tonumber. A producer test that resolves every per-component import against shared exports would catch this class of bug. - Consumer install weight:
ts-morphis a production dependency although its only code importer is the Loom build helperbuildTargetDts.ts. Moving it to devDependencies avoids approximately 14 MB unpacked in a fresh dependency installation (incremental savings depend on deduplication). Also consider excluding unexported docs-input aggregates from the publishedsrc/tree, after validating the packed file list and IDE/source-map expectations. Neither is browser-JS weight. - Destination handling: reuse the existing
resolveShopifyDevPathhelper rather than choosing~/world/trees/root/srcjust because it exists. Prefer explicit synchronization separate from generation so an alternate worktree or missing shopify-dev zone does not receive unintended writes. - Shared docs fix: the committed Checkout/customer-account artifacts still contain the same native-member expansion (about 9.8/10.8 MB, with 60/68 declarations exposing native sentinels). A collector-level option to exclude inherited native-DOM members by declaration origin, while preserving authored members, would be more durable than independent regexes in each surface. Keep the output invariant and test it. These are follow-ups, not a request to expand this PR to all surfaces.
Validation: isolated docs generation and positive/negative guard probes, targeted type probes, and inspection of the green CI checks. I did not rerun the full build/test suite or the wrapper that automatically copies into World.
| */ | ||
| error: AggregateError<T>; | ||
| error: AggregateError$1<T>; |
There was a problem hiding this comment.
from Kyle's agent review
Docs regression: can we restore a resolvable AggregateError docs entry before publishing this output? The base payload documented AggregateError.errors; all three head payloads now reference AggregateError$1<T> here, but contain neither an AggregateError nor an AggregateError$1 entry. This affects the FunctionSettings error documentation, so this particular removal is not just internal scaffolding cleanup.
The public-doc contract in the Polaris producer matches the unsuffixed name and misses the bundler-renamed declaration. Please normalize the alias/references and public-doc metadata consistently upstream, regenerate, and add a small reference-completeness fixture. The runtime API itself has not changed.
There was a problem hiding this comment.
from Kyle's agent review
Fixed in 4f01817. The regenerated aggregate and all three docs payloads now define public AggregateError.errors and AggregateErrorEvent.error references AggregateError<T>. The producer regression test is in the linked Polaris follow-up: https://meteorite.shopify.io/repos/shop/world/pulls/2062914
| appHomeGeneratedDocsDataV2Path, | ||
| ]); | ||
|
|
||
| await assertNoDomSurfaceLeak([ |
There was a problem hiding this comment.
from Kyle's agent review
Can we make temporary-file cleanup unconditional now that this intentionally throws on invalid output? fs.rm(tempComponentDefs) only runs at the end of the successful path. A rejected payload leaves src/surfaces/admin/components.ts behind; that file matches the package's TypeScript inputs and is under its published src/ tree. A scratch reproduction shows the next type-check fails on duplicate declarations/missing modules.
Move removal into finally and have copyGeneratedToShopifyDev return instead of calling process.exit() when no destination exists, since that also bypasses cleanup. Set process.exitCode at the entrypoint on failure. The lifecycle issue predates this PR, but this new deliberate failure path exposes it. Staging output before replacing the existing generated directory would also avoid partially written results.
| '@shopify/ui-extensions': minor | ||
| --- | ||
|
|
||
| Regenerate the Admin component definitions. Adds the missing `AppNav`, `EmptyState`, `Number`, `PressButton`, and `ScrollBox` types, plus `ProgressProps` in `shared.d.ts`. `Section` now accepts an `Icon` in its `graphic` slot and a `Tooltip` among its accessories. |
There was a problem hiding this comment.
from Kyle's agent review
Can we distinguish restored aggregate/docs coverage from newly available extension components here? AppNav.d.ts, EmptyState.d.ts, and Number.d.ts already existed at the base revision. PressButton and ScrollBox are restored only in the aggregate: this PR adds no public per-component export files or supported target-set entries for them. The current wording suggests consumers can start importing/using them.
The important consumer fix worth highlighting is that the missing shared ProgressProps definition now restores useful typing for Progress properties. I'd describe that fix, the Section/field documentation changes, and regeneration from Polaris 2.26.0 rather than advertising these five as new public component types. Keeping the minor changeset is appropriate.
There was a problem hiding this comment.
from Kyle's agent review
Fixed in 4f01817. The changeset now calls out the repaired ProgressProps typing and Admin docs regeneration, without advertising aggregate-only PressButton or ScrollBox definitions as publicly usable components.
| }); | ||
| await replaceFileContent({ | ||
| filePaths: tempComponentDefs, | ||
| searchValue: /declare const (\w+): \{[^{}]*\bHTMLElement\b[^{}]*\};/g, |
There was a problem hiding this comment.
from Kyle's agent review
\w+ does not include $, so the object-literal form stops matching if Rollup emits a normal deconflicted name such as BaseClass$2. The previous aggregate already used a $-suffixed native-base identifier. The sentinel correctly fails closed, but an ordinary naming variation would unnecessarily break docs generation.
Can we accept emitted JavaScript identifiers here (at minimum [\w$]+) and add fixtures for both historical forms, $ suffixes, whitespace variations, and an unrelated object that merely contains an HTMLElement-typed field? The last case also guards against over-neutralizing with the broad [^{}]*HTMLElement[^{}]* match. Extracting the rewrite and leak detection into importable pure helpers would let us test the negative case without running the whole script.
|
|
||
| const assertNoDomSurfaceLeak = async (filePaths) => { | ||
| for (const filePath of filePaths) { | ||
| const docsData = JSON.parse(await fs.readFile(filePath, 'utf8')); |
There was a problem hiding this comment.
from Kyle's agent review
Measured docs-size opportunity: there are still 766 isPrivate: true member records originating in component declarations, including repeated rendering internals and TypeScript symbol IDs. A scoped pruning probe reduces each JSON from 1,683,007 to 1,363,054 bytes (19% raw; 171,881 → 152,806 bytes with gzip). This does not remove public inherited props, raw declaration strings, or dependent types. Please confirm the docs renderer does not need those private records before landing the pruning.
Rather than adding another file pass, could we make normalization/validation/pruning one pure in-memory pipeline? Optionality already parses and rewrites these files, and this function opens/parses them again. We also run the same TypeScript harvester twice over the same input even though all three current outputs are byte-identical. Generate once, then derive the destination files with any target-specific URL transformations preserved. This is docs/build overhead, not runtime bundle weight.
| * @default 'start' | ||
| icon?: | ||
| | ( | ||
| | 'adjust' |
There was a problem hiding this comment.
from Kyle's agent review
Polaris producer follow-up: can we preserve a reusable IconType alias instead of expanding the full icon-name union at each use? The 563-name union appears seven times in this aggregate, including unreferenced alias declarations. Roughly 70 KB of the aggregate's 150 KB increase comes from this duplication; the restored components account for only about 13 KB. The same expansion also adds weight to the docs metadata.
Shared declarations already had this emit pattern at the base revision, so this is pre-existing producer behavior newly visible in the regenerated aggregate, not a new runtime cost. Fix the declaration output upstream and regenerate rather than manually rewriting the union here.
|
|
||
| /* eslint-disable @typescript-eslint/ban-types */ | ||
| /* eslint-disable @typescript-eslint/no-namespace */ | ||
| /* eslint-disable @typescript-eslint/member-ordering */ | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/triple-slash-reference, spaced-comment | ||
| /// <reference lib="DOM" /> | ||
| import * as preact$1 from 'preact'; | ||
| import {ReactNode, RefAttributes} from 'react'; | ||
| import * as _shopify_admin_web_component_foundations from '@shopify/admin-web-component-foundations'; |
There was a problem hiding this comment.
from Kyle's agent review
Pre-existing public-types issue, newly visible in this aggregate: this namespace import escapes the generator even though the public SDK does not depend on the restricted @shopify/admin-web-component-foundations package. The same import already exists at the base revision in the publicly exported components/Table.d.ts:18 and components/Clickable.d.ts:23; consumers checking declarations with skipLibCheck: false get TS2307. The aggregate itself is not a normal public entrypoint, so this is not a newly introduced consumer blocker in this PR.
The producer's import extraction handles named imports but misses namespace imports synthesized for inferred types. Please route namespace imports/qualified references through the same inlining and symbol-deconfliction process, and add an assertion that restricted-package imports cannot survive public emission. This belongs in a Foundations follow-up; adding the private runtime package as a dependency of the public SDK would be the wrong fix.
|
|
||
| declare const internals$2: unique symbol; | ||
| declare class BaseClass$2 extends PreactCustomElement { | ||
| declare class BaseClass extends PolarisCustomElement { |
There was a problem hiding this comment.
from Kyle's agent review
Polaris producer follow-up: this class collides with declare const BaseClass at line 7036, the native HTMLElement constructor base emitted by the other bundler. TypeScript reports TS2451 at both declarations. After the docs rewrite makes the first declaration any, ColorPicker resolves against that declaration and loses eight inherited member records. Those records are all private today, so no public component props regress, but the generated input is order-dependent.
Can the declaration merge deconflict symbols across its independently emitted halves and reject duplicate non-mergeable top-level declarations? The native-DOM sentinel cannot detect authored inheritance silently disappearing. Renaming one local base can mitigate this instance, but a producer invariant is the durable fix.
Assisted-By: devx/46533292-b415-41e6-a32e-f79b9737e793
Background
The Admin
components.d.tsaggregate had been generated for a subset of components and then patched by hand, so it had drifted from the components as shipped:AppNav,EmptyState,Number,PressButtonandScrollBoxwere missing entirely, and internal generator scaffolding had leaked in.Solution
Regenerated the definitions for the full component set, with no component filter, and rebuilt the
2026-10-rcdocs payload. Definitions move from Polaris 2.23.0 to 2.26.0:shared.d.tsgainsProgressPropsand generalises the field documentation inherited byCheckbox,Choice,ChoiceList,OptionandSwitch;Sectiondocuments anIconin its graphic slot andTooltipamong its accessories.Everything here is generated output.
🎩
yarn lintandyarn type-checkpass.Checklist