aws / jsii

jsii allows code in any language to naturally interact with JavaScript classes. It is the technology that enables the AWS Cloud Development Kit to deliver polyglot libraries from a single codebase!
https://aws.github.io/jsii
Apache License 2.0
2.63k stars 244 forks source link

JSII3000 (unexported type) error for a type that comes from a submodule of a JSII peer dependency #3095

Open ansgarm opened 2 years ago

ansgarm commented 2 years ago

:bug: Bug Report

Affected Languages

General Information

What is the problem?

With our 0.7 release of CDKTF we switched the provider generation for the AWS provider to use JSII submodules. This change brought up an issue in a custom constructs that makes use of those pre-built provider bindings: Exported APIs cannot use un-exported type "@pahud/cdktf-aws-eks.EKS.EksCluster" (originally reported here: https://github.com/hashicorp/cdktf-provider-aws/issues/465)

When exporting the namespace like this in the pre-built AWS provider:

export { EKS } from './EKS';

instead of

export * from './EKS';

JSII can compile the custom construct. However that also causes JSII to not treat those namespaces as submodules anymore (related comment) – hence I think it does not solve the underlying problem but rather rolls back the change.

So I dug a bit deeper and added a bit of logging to the _allTypeReferencesAreValid validator function. Apparently (and as visible in the error message) the typeRef causing the JSII3000 error is { fqn: '@pahud/cdktf-aws-eks.EKS.EksCluster' }. I also logged all assembly.types known to JSII: (full output)

{
  '@pahud/cdktf-aws-eks.CapacityType': {...},
  '@pahud/cdktf-aws-eks.ScalingConfig': {...},
  '@pahud/cdktf-aws-eks.NodeGroupOptions': {...},
  '@pahud/cdktf-aws-eks.NodeGroupProps': {...},
  '@pahud/cdktf-aws-eks.NodeGroup': {...},
  '@pahud/cdktf-aws-eks.ClusterProps': {...},
  '@pahud/cdktf-aws-eks.NodeGroupBaseOptions': {...},
  '@pahud/cdktf-aws-eks.Cluster': {...},
  '@pahud/cdktf-aws-eks.KubernetesVersion': {...}
}

And it appears that there is no namespace @pahud/cdktf-aws-eks.EKS and JSII somehow must be incorrectly inferring @pahud/cdktf-aws-eks.EKS.EksCluster for the EKS.EksCluster referenced here. It should probably be @cdktf/provider-aws.EKS.EksCluster or similar.

Could you point me to the logic that determines / resolves that name? My current suspicion is that it lacks support for submodules, but I'm only guessing here.

Reproduction example

https://github.com/ansgarm/jsii-namespace-peer-deps

RomainMuller commented 2 years ago

Instead of

export * from './EKS';

Have you tried

export * as EKS from './EKS';

I think what you're trying to do is reasonable and should work. I reckon it fell in an edge case of the submodule traversal logic, somehow... I need to attempt reproduction to understand where things go south...

RomainMuller commented 2 years ago

Would you be able to make a small repro repository for me to use? That'd help me get to this faster...

ansgarm commented 2 years ago

Hi @RomainMuller!

Sure! Here you go: https://github.com/ansgarm/jsii-namespace-peer-deps

Have you tried export * as EKS from './EKS';

Just tried it. Works, but is a bit less ideal due to the following reasons:

  1. JSII requires snake case for it. So only export * as eks from './EKS'; would work (this is something we could live with of course)
  2. As we are using namespaces in our submodules (see reproduction example also), usage would look like this:
    import { eks } from '@cdktf/provider-aws';
    const cluster = new eks.eks.EksCluster(…);

Edit: We are using TS namespaces, because we faced issues with too many exports causing RangeError: Maximum call stack size exceeded with the TypeScript version required by JSII. The AWS Terraform provider is too big 😅

skorfmann commented 2 years ago

FYI @RomainMuller: We tried a slightly different approach to namespaces today, and it looks like this might solve our immediate issue. Gonna get back to you with more details once I confirmed.

skorfmann commented 2 years ago

FYI @RomainMuller: We tried a slightly different approach to namespaces today, and it looks like this might solve our immediate issue. Gonna get back to you with more details once I confirmed.

This PR solved our issue around namespaces https://github.com/hashicorp/terraform-cdk/pull/1248

NGL321 commented 2 years ago

Hey @skorfmann,

I would like to confirm if this is still an issue in JSII or if it was confirmed to be resolved via the namespace change in tcdk?