Skip to content

fix(provider-generator): collapse unsupported nested collections to nearest Any wrapper#300

Open
so0k wants to merge 2 commits into
mainfrom
fix/issue-11-unsupported-nested-collection-classes
Open

fix(provider-generator): collapse unsupported nested collections to nearest Any wrapper#300
so0k wants to merge 2 commits into
mainfrom
fix/issue-11-unsupported-nested-collection-classes

Conversation

@so0k

@so0k so0k commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #11

Problem

Running cdktn get for the awscc provider generates bindings that fail to compile:

.gen/providers/awscc/data-awscc-appintegrations-data-integration/index.ts(327,44): error TS2551: Property 'StringListMapMap' does not exist on type

The generator composes stored-class names recursively (stringstringListstringListMapstringListMapMap for map(map(list(string)))) and emits new cdktn.StringListMapMap(...) for computed attributes — but the core cdktn package only hand-writes a fixed set of collection wrapper classes, so triple-nested combinations reference classes that don't exist. Upstream's attempt (hashicorp/terraform-cdk#3922, closed unmerged) skipped the attribute entirely.

Fix: collapse to the nearest typed Any wrapper

feat(lib) — adds NumberMapMap, BooleanMapMap (typed depth-2 shapes that were also broken; mirrors of the existing StringMapMap) and AnyMapMap. AnyMapMap closes the set: any collection nesting of any depth now collapses to one of AnyListList / AnyMapList / AnyListMap / AnyMapMap, which all exist. Additive JSII API.

fix(provider-generator)resolveStoredClassName() checks the composed name against the verified set of core classes; when unsupported it collapses to the nearest Any wrapper that still encodes the outer two collection layers. The any boundary sits exactly where core's class coverage ends, preserving typed navigation above it:

di.objectConfiguration          // AnyMapMap — typed, autocompleted
  .lookup("SourceObject")       //  → AnyMap
  .lookup("Fields")             //  → any token (the list(string) layer)

Collapsed getters carry a JSDoc block so the full shape stays visible in IntelliSense (attribute getters previously had no JSDoc at all):

/**
 * Terraform type: `map(map(list(string)))`.
 * This nesting is deeper than the typed wrapper classes in the core cdktn package; it is exposed as `AnyMapMap` and values below that boundary are untyped tokens.
 */
public get mapMapListString() { ... }

Supported shapes are untouched (map(list(string)) still emits cdktn.StringListMap, no JSDoc added — all pre-existing snapshots pass byte-for-byte), and input/setter types keep their full precision. If no stored class can be resolved at all — should be unreachable now — the getter falls back to a plain untyped interpolation getter, kept loud by a direct unit test on the branch plus a debug-log breadcrumb (attribute name + composed type) so a wild firing leaves evidence instead of a silent typing downgrade.

Note on version alignment: bindings generated by this CLI reference the new core classes, so cdktn and cdktn-cli must be on the same release (the standard support contract — first troubleshooting step for post-upgrade tsc errors is aligning the two).

Worked examples (all covered by the new fixture/snapshot test)

Terraform type Emitted class
map(map(list(string))) (awscc repro) AnyMapMap (collapsed)
list(map(map(string))) AnyMapList (collapsed)
set(map(list(number))) AnyMapList (collapsed, set-wrapped)
map(map(map(list(string)))) AnyMapMap (collapsed)
map(map(number)) / map(map(bool)) NumberMapMap / BooleanMapMap (newly typed)
map(list(string)) (control) StringListMap (unchanged)

Validation

  • pnpm nx build cdktn (tsc + jsii): clean
  • pnpm nx test @cdktn/provider-generator: 18/18 suites, 84/84 tests, 89/89 snapshots
  • pnpm nx test cdktn: 443/444 (the one failure is the pre-existing toPlanSuccessfully matcher test requiring a live terraform plan with Docker; fails identically on main)
  • New core unit tests mirror the existing StringMapMap lookup tests for the three new classes

🤖 Generated with Claude Code

Mirrors of the existing StringMapMap for computed map(map(number)) and
map(map(bool)) attributes, plus AnyMapMap which closes the wrapper-class
set for arbitrarily deep collection nestings: any unsupported shape can
now be collapsed to a nearest typed Any-wrapper (AnyListList, AnyMapList,
AnyListMap or AnyMapMap) by the provider generator.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@so0k so0k force-pushed the fix/issue-11-unsupported-nested-collection-classes branch from c3b468e to e773583 Compare July 3, 2026 12:37
@so0k so0k changed the title fix(provider-generator): fall back to plain getter for unsupported nested collection classes fix(provider-generator): collapse unsupported nested collections to nearest Any wrapper Jul 3, 2026
@so0k

so0k commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

If no stored class can be resolved at all — should be unreachable now — the getter falls back to a plain untyped interpolation getter, kept loud by a direct unit test on the branch plus a debug-log breadcrumb (attribute name + composed type) so a wild firing leaves evidence instead of a silent typing downgrade.

This can happen if a newer cdktn-cli is used to generate provider bindings consumed by an older cdktn core package - the debug-log should flag this (and fix would be to align all package versions).

Actual fallback scenarios to plain untyped interpolation getter (it's not version skew!):

Allowlist and collapse rule are hand-maintained invariants, not enforced ones

  • SUPPORTED_STORED_CLASSES is a static list that must mirror what core actually ships, and
  • the Any<inner><own> naming rule must cover every shape the parser can produce.

Both can drift within a single release: someone removes or renames a wrapper class in core and updates the list, a refactor breaks resolveStoredClassName, or a future Terraform schema feature introduces an element typeModelType the collapse doesn't map (the schema format has grown new shapes before — nested attribute types, dynamic, write-only attributes).

The guard is the difference between those mistakes shipping as compile-broken generated bindings in users' .gen folders — the exact issue #11 failure mode — versus a degraded-but-compiling untyped getter.

The cost asymmetry favors keeping it

The fallback isn't a subsystem; it's a conditional routing to the emitter's pre-existing plain getter path (used today for tokenizable and assignable attributes). Deleting it saves a few lines. Keeping it converts an entire class of future bugs from "provider unusable, user's build breaks" into "one attribute reads as an untyped token, with a breadcrumb."

The alternative failure modes are worse

Without the guard, an unresolved shape either emits a non-existent class (silent until the user's tsc run) or you make generation throw — which turns one odd attribute into "cdktn get fails for the whole provider," precisely what issue #11 complains about.

So the honest framing: it's cheap insurance against our own future mistakes and unknown future schema shapes — not against version skew, which only the consumer's compiler (and the lockstep versioning contract) can catch. If we judge that insurance not worth one conditional plus one test, removal is a two-line change

Comment on lines +2244 to +2250
/**
* Terraform type: \`map(map(list(string)))\`.
* This nesting is deeper than the typed wrapper classes in the core cdktn package; it is exposed as \`AnyMapMap\` and values below that boundary are untyped tokens.
*/
public get mapMapListString() {
return this._mapMapListString;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collapsed AnyMapMap JSDocs indicate original terraform type

…earest Any wrapper

Computed attributes whose type nests list/set/map deeper than the fixed
set of wrapper classes hand-written in the core cdktn package (e.g. the
awscc provider's map(map(list(string)))) composed stored-class names
like "StringListMapMap" that don't exist, producing bindings that fail
to compile:

  error TS2551: Property 'StringListMapMap' does not exist on type ...

The generator now resolves the composed name against the set of classes
that actually exist in core; when unsupported, it collapses to the
nearest typed Any-wrapper that still encodes the outer two collection
layers (map(map(list(string))) => AnyMapMap), preserving typed
navigation down to where core's class coverage ends instead of skipping
the attribute (the approach of the closed upstream
hashicorp/terraform-cdk#3922) or degrading to a fully opaque token.

Collapsed getters carry a JSDoc block stating the full Terraform type
and the untyped boundary, so the shape stays visible in IntelliSense
without a docs round-trip. Input/setter types are unaffected. If no
stored class can be resolved at all (should be unreachable), the getter
falls back to a plain untyped interpolation getter, guarded by a direct
unit test and a debug-log breadcrumb.

Fixes #11

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@so0k so0k force-pushed the fix/issue-11-unsupported-nested-collection-classes branch from e773583 to 6f65fe3 Compare July 3, 2026 14:26
@sakul-learning

Copy link
Copy Markdown
Contributor

Review update

I re-checked the latest head (6f65fe3) after the force-push. The only delta from the previously reviewed head is the removal of the unused isStoredClassSupported export, which resolves the small cleanup note.

Validation I ran on the reviewed implementation:

  • pnpm exec jest packages/cdktn/test/validations.test.ts --runInBand — 31 tests passed, 9 snapshots passed
  • pnpm exec lerna run --scope cdktn build — successful
  • pnpm exec nx test cdktn --runInBand — 38 suites passed, 444 tests passed, 288 snapshots passed
  • Built/packaged local JS artifacts, generated an awscc 1.91.0 repro project, compiled it, synthesized it, and ran terraform validate against the generated cdk.tf.json — valid
  • Spot-checked yandex-cloud/yandex provider generation: 402 TS files / ~28MB / ~682k lines generated; no NumberMapMap, BooleanMapMap, or AnyMapMap usages appeared there, and the generated demo project still compiled

The AWSCC repro output now emits the expected collapsed wrapper for the problematic data source shape:

private _objectConfiguration = new cdktn.AnyMapMap(this, "object_configuration");

and the synthesized Terraform output uses the expected nested lookup expression:

data.awscc_appintegrations_data_integration.data_integration.object_configuration["example_object"]["fields"]

A couple of notes from the provider checks:

  • This path affects computed data-source attributes that need stored wrapper classes. Resource-side nested input shapes are inline TypeScript input shapes and synthesize through nested mapper functions, so they are not the same wrapper-class failure mode.
  • The checked providers did not require every newly added parity wrapper class, but adding NumberMapMap / BooleanMapMap beside the existing StringMapMap is reasonable API parity and keeps supported depth-2 shapes typed.
  • The generated-size check stayed effectively flat: awscc@1.91.0 changed by only +215 bytes / +4 TS lines, and aws@6.0.0 was byte-for-byte unchanged in generated bindings, so this does not appear to regress the existing skipped-attribute size mitigation.

Given the validations and spot checks above, I do not think additional follow-up work is needed for this PR at this stage. Looks good to ship.

@so0k

so0k commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

feat(lib) — adds NumberMapMap, BooleanMapMap (typed depth-2 shapes that were also broken; mirrors of the existing StringMapMap)

this addresses #110 (comment) - but spot checks didn't show any provider (awcc, yandex) using them

Comment on lines +13 to +35
const SUPPORTED_STORED_CLASSES = new Set([
"StringMap",
"NumberMap",
"BooleanMap",
"AnyMap",
"StringMapMap",
"NumberMapMap",
"BooleanMapMap",
"AnyMapMap",
"BooleanList",
"StringListList",
"NumberListList",
"BooleanListList",
"AnyListList",
"StringMapList",
"NumberMapList",
"BooleanMapList",
"AnyMapList",
"StringListMap",
"NumberListMap",
"BooleanListMap",
"AnyListMap",
]);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optionally we add a test which ensures this list matches ../../../../../../cdktn/src/complex-computed-list.ts

# packages/@cdktn/provider-generator/src/get/__tests__/generator/supported-stored-classes.test.ts
  // Hand-written wrapper classes follow the naming convention
  // <String|Number|Boolean|Any><Map|List>(<Map|List>)* (e.g. StringMap, AnyMapMap,
  // StringListMap). Other exported classes in that file (ComplexComputedList,
  // ComplexObject, and the abstract ComplexList/ComplexMap/MapList base classes) are
  // structural helpers, not stored-class wrappers, and are intentionally excluded by
  // this pattern rather than by name, so newly added wrapper classes are picked up
  // automatically.
  const WRAPPER_CLASS_NAME = /^(?:String|Number|Boolean|Any)(?:List|Map)+$/;

  function getHandWrittenWrapperClasses(): string[] {
    const source = readFileSync(
      join(
        __dirname,
        "../../../../../../cdktn/src/complex-computed-list.ts",
      ),
      "utf-8",
    );

    const classNames: string[] = [];
    const classDeclaration = /^export (?:abstract )?class (\w+)/gm;
    let match: RegExpExecArray | null;
    while ((match = classDeclaration.exec(source))) {
      classNames.push(match[1]);
    }

    return classNames.filter((name) => WRAPPER_CLASS_NAME.test(name));
  }

  test("every hand-written wrapper class is registered in SUPPORTED_STORED_CLASSES", () => {
    const wrapperClasses = getHandWrittenWrapperClasses();

    // Sanity-check the extraction itself so a change to complex-computed-list.ts that
    // breaks the parser (rather than the coverage) fails loudly and specifically.
    expect(wrapperClasses.length).toBeGreaterThan(0);

    const missing = wrapperClasses.filter(
      (name) => !SUPPORTED_STORED_CLASSES.has(name),
    );
    expect(missing).toEqual([]);
  });

  test("SUPPORTED_STORED_CLASSES has no entries that don't correspond to a real class", () => {
    const wrapperClasses = new Set(getHandWrittenWrapperClasses());

    const stale = [...SUPPORTED_STORED_CLASSES].filter(
      (name) => !wrapperClasses.has(name),
    );
    expect(stale).toEqual([]);
  });

Cons:

  1. Regex-on-source parsing is format-coupled. It relies on prettier's stable export class Name single-line form. Acceptable: prettier enforces that shape repo-wide, the length > 0 sanity check catches parser rot in one direction, and a totally broken extraction would make all 21 entries look "stale" and fail the other direction too. A file rename/split in core fails with ENOENT rather than a descriptive message — loud, just less self-explanatory.
  2. A future wrapper with a divergent constructor signature would pass parity and only be caught if its shape appears in the fixture snapshot. Given all 21 current classes conform and new ones are written by mirroring, I'd not add machinery for this now — worth a line in the test's comment at most.
  3. Cross-package reach. The test reads another workspace's source file, which is slightly unusual coupling — but it's precisely the coupling the invariant is about, so encoding it explicitly is honest. The alternative (importing the built cdktn package and reflecting on exports) would be stronger (it would also catch a dropped index re-export) but requires the core build artifact to exist before provider-generator unit tests run; the source-read approach has no build-order dependency. Reasonable trade-off as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provider bindings reference non-existent StringListMapMap

2 participants