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

JSII doesnt genereate nullable enum in .Net #414

Closed
McDoit opened this issue Mar 29, 2019 · 3 comments · Fixed by #432
Closed

JSII doesnt genereate nullable enum in .Net #414

McDoit opened this issue Mar 29, 2019 · 3 comments · Fixed by #432
Assignees
Labels
bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) p0

Comments

@McDoit
Copy link

McDoit commented Mar 29, 2019

The following code in the .net CDK

  [JsiiByValue]
  public class SubnetSelection : ISubnetSelection
  {
    [JsiiProperty("subnetName", "{\"primitive\":\"string\",\"optional\":true}", true)]
    public string SubnetName { get; set; }

    [JsiiProperty("subnetType", "{\"fqn\":\"@aws-cdk/aws-ec2.SubnetType\",\"optional\":true}", true)]
    public SubnetType SubnetType { get; set; }
  }

doesnt follow the nullability in vpc-ref.ts

export interface SubnetSelection {
  /**
   * Place the instances in the subnets of the given type
   *
   * At most one of `subnetType` and `subnetName` can be supplied.
   *
   * @default SubnetType.Private
   */
  readonly subnetType?: SubnetType;

  /**
   * Place the instances in the subnets with the given name
   *
   * (This is the name supplied in subnetConfiguration).
   *
   * At most one of `subnetType` and `subnetName` can be supplied.
   *
   * @default name
   */
  readonly subnetName?: string;
}

Which makes .Net code throw Only one of subnetType and subnetName can be supplied everytime a SubnetName is used, even if no value is defined for SubnetType

https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts

@McDoit
Copy link
Author

McDoit commented Apr 2, 2019

Same problem with SslPolicy in ApplicationListenerProps

  [JsiiByValue]
  public class ApplicationListenerProps : IApplicationListenerProps, IBaseApplicationListenerProps
  {
    [JsiiProperty("loadBalancer", "{\"fqn\":\"@aws-cdk/aws-elasticloadbalancingv2.IApplicationLoadBalancer\"}", true)]
    public IIApplicationLoadBalancer LoadBalancer { get; set; }

    [JsiiProperty("certificateArns", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"primitive\":\"string\"}},\"optional\":true}", true)]
    public string[] CertificateArns { get; set; }

    [JsiiProperty("defaultTargetGroups", "{\"collection\":{\"kind\":\"array\",\"elementtype\":{\"fqn\":\"@aws-cdk/aws-elasticloadbalancingv2.IApplicationTargetGroup\"}},\"optional\":true}", true)]
    public IIApplicationTargetGroup[] DefaultTargetGroups { get; set; }

    [JsiiProperty("open", "{\"primitive\":\"boolean\",\"optional\":true}", true)]
    public bool? Open { get; set; }

    [JsiiProperty("port", "{\"primitive\":\"number\",\"optional\":true}", true)]
    public double? Port { get; set; }

    [JsiiProperty("protocol", "{\"fqn\":\"@aws-cdk/aws-elasticloadbalancingv2.ApplicationProtocol\",\"optional\":true}", true)]
    public ApplicationProtocol Protocol { get; set; }

    [JsiiProperty("sslPolicy", "{\"fqn\":\"@aws-cdk/aws-elasticloadbalancingv2.SslPolicy\",\"optional\":true}", true)]
    public SslPolicy SslPolicy { get; set; }
  }

https://github.com/awslabs/aws-cdk/blob/master/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

@rix0rrr rix0rrr added bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) labels Apr 2, 2019
@fulghum fulghum added the p0 label Apr 2, 2019
RomainMuller added a commit that referenced this issue Apr 4, 2019
Method `Parameters` now carry an `optional` flag that indicates whether
they are optional or required, and the `TypeReference#optional` field
was renamed to `TypeReference#nullable` to better reflect its semantics.

This also brings more flexibility in that it is now possible to model a
method with a nullable or defaulted argument that is followed by some
non-optional argument, and still obtain a reasonable type specification,
where previously this was an error.

Finally, in order to better reflect the type model of TypeScript and
Javascript, all `any` type references are now denoted `nullable`.

BREAKING CHANGE: JSII assemblies generated by older versions of the tool
will fail loading with this new version, and vice-versa. Re-compile your
projects in order to fix this.

Fixes #296
Fixes #414
@fulghum
Copy link

fulghum commented Apr 4, 2019

@RomainMuller – I moved this from the CDK board to the (new) .NET board. It sounds like there's .NET specific work after the jsii core work?

@RomainMuller
Copy link
Contributor

Well. There was an issue in the .Net code generator that I have fixed as part of my work in #432. Once this ships, this bug should be gone.

RomainMuller added a commit that referenced this issue Apr 10, 2019
Method `Parameters`, `Properties` and method return types
now carry an `optional` flag that indicates whether they are
optional or required, and the `TypeReference#optional` field
was removed.

BREAKING CHANGE: JSII assemblies generated by older versions of the tool
will fail loading with this new version, and vice-versa. Re-compile your
projects in order to fix this.

Fixes #296
Fixes #414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. language/dotnet Related to .NET bindings (C#, F#, ...) p0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants