fix(s3): make Bucket assignable to IBucket under exactOptionalPropertyTypes - #38104
fix(s3): make Bucket assignable to IBucket under exactOptionalPropertyTypes#38104laazyj wants to merge 2 commits into
Conversation
…yTypes `IBucket.isWebsite` is declared optional (`?: boolean`), but `Bucket` exposed it through a getter typed `boolean | undefined` (delegating to `BucketReflection`). Under TypeScript's `exactOptionalPropertyTypes`, a member typed `T | undefined` is not assignable to an optional `?: T`, so consumers who enable the flag get a TS2375 error when passing a `Bucket` where an `IBucket` is expected (e.g. `BucketProps.serverAccessLogsBucket`), and type-checking `aws-cdk-lib` fails for them outright. The library does not build with the flag, so the break is invisible to its own compile. Reported by a user on aws#37996. `Bucket` satisfies `IBucket` through `BucketBase`, which declares `abstract readonly isWebsite?: boolean`, so there is no TS2420 at the class — the break only surfaces as a call-site assignment failure, which a class-level scan misses. Convert `isWebsite` (on `Bucket` and the internal `fromCfnBucket` subclass) from a getter to a `public readonly isWebsite?: boolean` field, set in the constructor from `this.reflection.isWebsite` once the underlying `CfnBucket` (with its website configuration) exists. Narrowing the getter to `: boolean` is not an option — jsii rejects a required getter overriding the optional abstract member (JSII5009) — so the field is the shape that satisfies both jsii and the flag. The property stays optional and immutable in the jsii assembly, so this is not an API change and `yarn compat` passes. Website hosting is configured by props at construction, so capturing the value there is behaviour-preserving. Add a package-local regression guard. This PR also introduces a shared, auto-enumerating helper in `@aws-cdk/cdk-build-tools`, `exactOptionalAssignabilityDiagnosticsForPackage(testDir)`, which enumerates every exported, non-abstract class in a package and type-checks its assignability to each interface it implements (own and inherited) under the flag — so the guard is complete by construction and catches a newly-added offending class automatically, rather than relying on a hand-maintained list. Refs aws#37996
|
Exemption Request: This is a type-only change — it converts an existing read-only getter ( |
aws-cdk-automation
left a comment
There was a problem hiding this comment.
The pull request linter fails with the following errors:
❌ Fixes must contain a change to an integration test file and the resulting snapshot.
If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.
✅ A exemption request has been requested. Please wait for a maintainer's review.
Issue # (if applicable)
Refs #37996 (reported by @mingkunm on the issue).
Reason for this change
IBucket.isWebsiteis declared optional (?: boolean), butBucketexposed it through a getter typedboolean | undefined(delegating toBucketReflection). Under TypeScript'sexactOptionalPropertyTypes, a member typedT | undefinedis not assignable to an optional?: T, so consumers who enable that flag hit aTS2375error when passing aBucketwhere anIBucketis expected (e.g.BucketProps.serverAccessLogsBucket), and type-checkingaws-cdk-libfails for them outright:aws-cdk-libdoes not build with the flag, so the break is invisible to the library's own compile. Notably,BucketsatisfiesIBucketthroughBucketBase(which declaresabstract readonly isWebsite?: boolean), so there is noTS2420at the class — the break only surfaces as a call-site assignment failure. This is one package in the sweep tracked by #37996.Description of changes
Convert
isWebsite(onBucketand the internalfromCfnBucketsubclass) from a getter to apublic readonly isWebsite?: booleanfield, set in the constructor fromthis.reflection.isWebsiteonce the underlyingCfnBucket(with its website configuration) exists.: booleanis not an option — jsii rejects a required getter overriding the optional abstract member (JSII5009). Thereadonlyfield is the shape that satisfies both jsii and the flag.optionalandimmutablein the jsii assembly, so this is not an API change;yarn compatpasses with no new entry.Bucketis configured by props at construction (there is no post-construction L2 API to enable it), so capturing the value at construction is behaviour-preserving for supported usage. The existingisWebsiteunit tests are unchanged and pass.New: an auto-enumerating regression guard
This PR introduces a shared helper in
@aws-cdk/cdk-build-tools,exactOptionalAssignabilityDiagnosticsForPackage(testDir), which enumerates every exported, non-abstract class in a package and type-checks its assignability to each interface it implements (its own and inherited) under the flag. Enumerating — rather than hand-listing classes — keeps the guard complete by construction: a newly-added offending class (or a new optional getter on an existing one) is caught automatically. Theaws-s3guard test is a one-liner over this helper, and subsequent sweep packages can adopt it the same way.Description of how you validated your changes
aws-s3/test/exact-optional-property-types.test.ts) passes; verified it goes red with the precise diagnosticBucket -> IBucket: TS2375 [isWebsite]if the fix is reverted.isWebsitebehaviour tests (defaultfalse,truefor each website configuration) are unchanged and pass; fullaws-s3suite green.yarn compatpasses (no assembly change); lint clean.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license