(aws-cdk-lib/core): private->protected for CfnMapping.mapping and more

[athewsey]

Describe the feature

CfnMapping.mapping (and maybe several other props?) would be less restrictive on derived constructs if they were defined as protected rather than private.

More generally, I suspect this is a widespread issue and a lot of constructs might benefit from loosening up private properties?

Use Case

I'm trying to derive a child class from CfnMapping for a specific family of mappings where key 1 is AWS region and key 2 is an enumeration specific to my application.

My construct has pre-defined data for a wide variety of regions, but users can pass in an regions prop at construction to filter this down to keep the generated template small.

I'd like to offer convenience methods on the construct that expose information about the map as initialized: For example which regions are currently supported.

I would prefer to take this information directly from the .mapping in case it's been modified by any other method (like CfnMapping.setValue())... But because the property is defined as private my child class can't read it.

Today it seems like my options are either 1/ to hack around the type system to convince TS to do what I want and linters to ignore my hubris, or 2/ re-implement the base construct myself?

Proposed Solution

I suggest to make CfnMapping.mapping a protected property rather than private, and maybe open a broader discussion on whether protected or private should be preferred in general.

I appreciate this could create a perceived grey area between what is and isn't "a public interface" and "a breaking change" for constructs: and propose to take the stance that protected members should be considered unstable (like private ones) and subject to breaking change in any release.

Nevertheless, defining such members as protected over private at least gives construct authors the choice to build on them - with encouragement that pinning CDK version and/or rigorously testing on any upgrades is a best-practice.

Other Information

Today, I couldn't see any explicit direction in the design guidelines on the use of protected vs private in constructs. There's this very old issue but I couldn't parse a clear direction on the subject from it.

In some less strictly-typed languages (like Python), I believe users would already be able to access these properties pretty straightforwardly.

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

CDK version used

2.87.0, but consistent with current main branch

Environment details (OS name and version, etc.)

macOS, projen, cdklabs-projen-project-types/cdklabs-construct-lib

[ashishdhingra]

Needs discussion with the team.

[TheRealAmazonKendra]

In general, I don't see this as a direction we want to go but I'd like to better understand the use case for this.

and propose to take the stance that protected members should be considered unstable (like private ones) and subject to breaking change in any release.

We define stability at the module level. Breaking encapsulation here and declaring that the implementation details of that broken encapsulation isn't stable isn't stable both breaks the model we've promised to customers to the last several years and will likely setup customers for pain down the road.

[athewsey]

I had a quick search for more general discussions on the topic, but so far come up mainly with voices that agree protected should generally be avoided:

• https://softwareengineering.stackexchange.com/questions/436521/are-there-any-legitimate-use-cases-for-protected-visibility
• https://softwareengineering.stackexchange.com/questions/312425/why-have-private-fields-isnt-protected-enough
• https://softwareengineering.stackexchange.com/questions/314192/best-practices-for-using-public-protected-private
• https://stackoverflow.com/questions/76194/should-protected-attributes-always-be-banned
• https://stackoverflow.com/questions/37011/should-you-ever-use-protected-member-variables

It seems like there's general support to draw no distinction between consumers of public APIs (i.e. consumers of constructs for us) and developers trying to subclass & customize.

My specific use-case as mentioned above was to check (at build-time but after construction) whether a particular AWS region & other key was supported by a generated CfnMapping. For now I'm working around it by simply removing the check, which is not ideal because failure would be at deploy time & take longer... But it's not worth the effort of re-implementing CfnMapping's functionality.

Closing this issue for now & I guess will re-raise a specific public API request to retrieve the info if it comes back to bite us in future. Thanks for looking into it!

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[aws-cdk-automation]

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.