Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
- Coverage 94.58% 94.40% -0.19%
==========================================
Files 11 14 +3
Lines 517 679 +162
Branches 143 175 +32
==========================================
+ Hits 489 641 +152
- Misses 28 38 +10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
14571cd to
279100f
Compare
There was a problem hiding this comment.
Pull request overview
Adds an internal API generator for creating self-contained, patched Cloudscape proxy interfaces.
Changes:
- Implements patch parsing, declaration transformation, dependency traversal, and removal markers.
- Exposes API-gen package subpaths and adds
ts-morph. - Adds comprehensive fixtures, snapshots, and tests.
Reviewed changes
Copilot reviewed 15 out of 46 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.ts |
Narrows coverage to source files. |
tsconfig.test.json |
Sets the test root directory. |
test/tsconfig.json |
Reuses shared test configuration. |
test/api-gen/to-proxy-source.test.ts |
Tests source cleanup transformations. |
test/api-gen/test-helpers.ts |
Provides API-gen fixture helpers. |
test/api-gen/generate-proxy-interfaces.test.ts |
Tests generation and validation behavior. |
test/api-gen/__snapshots__/generate-proxy-interfaces.test.ts.snap |
Captures generated interface output. |
src/api-gen/to-proxy-source.ts |
Transforms upstream declaration source. |
src/api-gen/resolve-package.ts |
Resolves packages and relative imports. |
src/api-gen/parse-patch.ts |
Parses augmentation patches and markers. |
src/api-gen/markers.ts |
Defines the removal marker. |
src/api-gen/index.ts |
Exposes the generator API. |
src/api-gen/generate-proxy-interfaces.ts |
Generates the proxy declaration tree. |
src/api-gen/apply-patch.ts |
Applies interface and namespace patches. |
package.json |
Adds exports and runtime dependency. |
package-lock.json |
Locks API-gen dependencies. |
fixtures/api-gen/valid/checkbox/override-props.patch.d.ts |
Tests checkbox overrides. |
fixtures/api-gen/valid/button/remove-property.patch.d.ts |
Tests property removal. |
fixtures/api-gen/valid/button/remove-member.patch.d.ts |
Tests member removal. |
fixtures/api-gen/valid/button/marker-imports.patch.d.ts |
Tests marker import forms. |
fixtures/api-gen/valid/button/empty.patch.d.ts |
Tests an empty button patch. |
fixtures/api-gen/valid/button/carry-imports.patch.d.ts |
Tests carried imports. |
fixtures/api-gen/valid/button/add-props.patch.d.ts |
Tests added properties. |
fixtures/api-gen/valid/button-alpha/interfaces.patch.d.ts |
Tests alternate proxy placement. |
fixtures/api-gen/valid/box/remove-nested-ns.patch.d.ts |
Tests nested namespace removal. |
fixtures/api-gen/valid/box/override-nested-ns.patch.d.ts |
Tests nested namespace replacement. |
fixtures/api-gen/valid/box/empty.patch.d.ts |
Tests an empty box patch. |
fixtures/api-gen/valid/box/carry-imports.patch.d.ts |
Tests imported box types. |
fixtures/api-gen/node_modules/@fixtures/upstream/types/events.d.ts |
Provides event type fixtures. |
fixtures/api-gen/node_modules/@fixtures/upstream/types/base-component.d.ts |
Provides base component fixtures. |
fixtures/api-gen/node_modules/@fixtures/upstream/internal/README.md |
Represents a missing declaration fixture. |
fixtures/api-gen/node_modules/@fixtures/upstream/icon/interfaces.d.ts |
Provides upstream icon declarations. |
fixtures/api-gen/node_modules/@fixtures/upstream/checkbox/interfaces.d.ts |
Provides upstream checkbox declarations. |
fixtures/api-gen/node_modules/@fixtures/upstream/button/interfaces.d.ts |
Provides upstream button declarations. |
fixtures/api-gen/node_modules/@fixtures/upstream/box/interfaces.d.ts |
Provides upstream box declarations. |
fixtures/api-gen/error/icon/place-taken-by-upstream.patch.d.ts |
Tests output-path collisions. |
fixtures/api-gen/error/demo/no-upstream.patch.d.ts |
Tests unresolved packages. |
fixtures/api-gen/error/demo/no-upstream-dts.patch.d.ts |
Tests missing declaration files. |
fixtures/api-gen/error/demo/no-augmentation.patch.d.ts |
Tests missing augmentations. |
fixtures/api-gen/error/button/remove-missing-property.patch.d.ts |
Tests invalid property removal. |
fixtures/api-gen/error/button/remove-missing-member.patch.d.ts |
Tests invalid member removal. |
fixtures/api-gen/error/button/no-upstream-namespace.patch.d.ts |
Tests unknown namespaces. |
fixtures/api-gen/error/button/no-upstream-interface.patch.d.ts |
Tests unknown interfaces. |
fixtures/api-gen/error/button/add-ns-value.patch.d.ts |
Tests rejected namespace values. |
fixtures/api-gen/error/button/add-ns-ns.patch.d.ts |
Tests rejected nested namespaces. |
fixtures/api-gen/error/button/add-ns-enum.patch.d.ts |
Tests rejected namespace enums. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| function carryOverImports(out: ts.SourceFile, patch: Patch) { | ||
| out.addImportDeclarations(patch.imports); | ||
| } |
There was a problem hiding this comment.
That is possible, but I decided not include it in scope just yet. We can add this reactively if there is ever a use case for it.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
avinashbot
left a comment
There was a problem hiding this comment.
Some minor comments about clarifying certain things, but that's all; none blocking. Great fixture coverage!
| const typeToMarker = new Map<string, Marker>(); | ||
| for (const declaration of patch.getImportDeclarations()) { | ||
| // Ignore any import other than from '…/api-gen/markers'. | ||
| if (declaration.getModuleSpecifierValue() !== MARKERS_MODULE) { |
There was a problem hiding this comment.
Do we ever expect a situation where the markers are imported from a location other than the npm-public package? This would effectively make @cloudscape-design/documenter package name part of a public API.
| } | ||
|
|
||
| export function resolveUpstreamModule(moduleSpecifier: string, fromDir: string): UpstreamFile { | ||
| const [scope, name, ...subpath] = moduleSpecifier.split('/'); |
There was a problem hiding this comment.
Perhaps a bit of an assumption that all our package will always have a scope? Realistically, it's probably safe, but maybe worth commenting.
| function patchInterfaces(out: ts.SourceFile, patch: Patch) { | ||
| for (const interfacePatch of patch.interfaces) { | ||
| const outInterface = findInterface(interfacePatch.interfaceName); | ||
| for (const override of removalsFirst(interfacePatch.propertyOverrides)) { |
There was a problem hiding this comment.
Why removals first? Clarifying comment would help.
| import { toProxySource } from './to-proxy-source'; | ||
|
|
||
| export interface UpstreamImport { | ||
| /** Path of the emitted file holding the import, e.g. `select/interfaces.ts`. */ |
There was a problem hiding this comment.
Thing that confused me a lot reading this file was not knowing what the strings could look like; maybe it's worth adding examples like this to the other path/location string props if possible?
Internal util to generate interfaces for proxy Cloudscape components.
Proposal: 92DfrTApyLG2
The new API is represented with
generateProxyInterfaces, which takes paths to patch files, and outputs a list of transformed files to write on the disk.A patch is a
.d.tsmodule augmentation next to the proxied component. Members are added orreplaced by declaring them, and removed by typing them with the
Removemarker:The build script points the generator at the patches and writes what it returns:
For the patch above that emits
button/interfaces.tswith variant narrowed and iconName gone, plus everything those interfaces reach — other components and shared types — so the generated tree resolves within itself. Along the way license headers and@awsuiSystemannotations are stripped and ambient namespaces are made real. An optionalresolveImporthook rewrites the specifiers of emitted imports, for consumers that re-export the shared types from their own paths.Patches that name something upstream does not declare fail the build instead of silently doing nothing, so an upstream rename surfaces as an error rather than a missing override.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.