Better Support for Mapped Types

biffgaut commented 3 years ago

:rocket: Feature Request

Affected Languages

[ ] TypeScript or Javascript
[ ] Python
[ ] Java
[ ] .NET (C#, F#, ...)
[ ] Go

General Information

JSII Version: 1.30.0
Platform: Linux docker-desktop 5.10.25-linuxkit #1 SMP Tue Mar 23 09:27:39 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
[ ] I may be able to implement this feature request
[ ] This feature might incur a breaking change

Description

We would like to make an attribute of an exported Properties object Partial<>, like this:

export interface MyLevelThreeConstructProps{
  readonly serviceProps: Partial<ServiceProps>
}

This converts ServiceProps to a Mapped Type that JSII does not support:

error JSII1003: Only string-indexed map types are supported

This is pretty deep inside the Typescript transpilation process, so I'm not sure of the exact nature of the Mapped Type that makes it incompatible. What are the chances of a fix to JSII that supports this sort of Mapped Type?

Proposed Solution

eriklz commented 3 years ago

I noticed this issue here because I get the same error message for a number of type declarations that looks like:

values: Record<string, SomeInterfaceType>

but not for

values: Record<string, string>

I converted these structures to

values: RecordWrap[]

where RecordWrap is

type RecordWrap = { name: string; data: T; };

but I get the same JSII1003 error on these as well. Also, using

values: { [key:string]: SomeInterfaceType; }

also generates the error, as expected.

A brief look into code seems that this might come from a call to _mapType() in assembler.ts, which in turn called from _optionalValue(), but haven't taken it any deeper than that now.

revmischa commented 2 years ago

I really would like to be able to use Partial<T> for allowing users to customize CDK resources generated internally in my construct https://github.com/jetbridge/cdk-nextjs

I can't even use Pick<T>, e.g.:

export interface NextjsDistributionCdkOverrideProps
  extends Pick<
    cloudfront.DistributionProps,
    // add to this if you want to override something
    'defaultRootObject' | 'errorResponses' | 'minimumProtocolVersion' | 'priceClass' | 'webAclId'
  > {}

Gives the error:

[2022-11-29T11:41:20.669] [ERROR] jsii/compiler - Type model errors prevented the JSII assembly from being created
src/NextjsDistribution.ts:27:11 - error JSII3004: Illegal extends clause for an exported API: MappedType

 27   extends Pick<
              ~~~~~
 28     cloudfront.DistributionProps,
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
... 
 30     'defaultRootObject' | 'errorResponses' | 'minimumProtocolVersion' | 'priceClass' | 'webAclId'
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 31   > {}
    ~~~

  node_modules/jsii/node_modules/typescript/lib/lib.es5.d.ts:1462:35
    1462 type Pick<T, K extends keyof T> = {
                                           ~
    1463     [P in K]: T[P];
         ~~~~~~~~~~~~~~~~~~~
    1464 };
         ~
    The invalid super type is declared here.

RomainMuller commented 2 years ago

Those types cannot be represented in non-TypeScript languages. We could imagine allowing named types to reduce the boilerplate involved in re-implementing those types (export type SomeName = Pick<X, '...'>), which jsii could implicitly convert to the expanded type here...

But it is worth noting that Java, C#, and Go resort to nominal typing, where TS uses structural typing... This means the "new type" would not be related to the other one in other languages, which likely will result in API usability problems in many situations...

Folks who are interested are welcome to draft an RFC to add support for these, however... as I think they can be powerful tools to reduce boilerplate.

aws / jsii