Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better Support for Mapped Types #2901

Open
7 tasks
biffgaut opened this issue Jul 7, 2021 · 3 comments
Open
7 tasks

Better Support for Mapped Types #2901

biffgaut opened this issue Jul 7, 2021 · 3 comments
Assignees
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. p1

Comments

@biffgaut
Copy link

biffgaut commented Jul 7, 2021

🚀 Feature Request

Affected Languages

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

General Information

  • 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
Copy link

eriklz commented Jul 23, 2021

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.

@peterwoodworth peterwoodworth added effort/medium Medium work item – a couple days of effort p1 and removed needs-triage This issue or PR still needs to be triaged. labels Jun 10, 2022
@revmischa
Copy link

revmischa commented Nov 29, 2022

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Medium work item – a couple days of effort feature-request A feature should be added or improved. p1
Projects
None yet
Development

No branches or pull requests

5 participants