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

Enums with duplicate values dropped #2782

Closed
njlynch opened this issue Apr 9, 2021 · 2 comments Β· Fixed by #3412
Closed

Enums with duplicate values dropped #2782

njlynch opened this issue Apr 9, 2021 · 2 comments Β· Fixed by #3412
Labels
bug This issue is a bug. effort/medium Medium work item – a couple days of effort module/compiler Issues affecting the JSII compiler p1

Comments

@njlynch
Copy link
Contributor

njlynch commented Apr 9, 2021

πŸ› Bug Report

Affected Languages

  • [ ] TypeScript or Javascript
  • [βœ… ] Python
  • [βœ… ] Java
  • [βœ… ] .NET (C#, F#, ...)
  • [❓ ] Go

General Information

  • JSII Version: 1.27.0
  • Platform: OSX

What is the problem?

Originally reported in aws/aws-cdk#14059.

It appears that repeated values (not names) in enums are ignored, leading to dropped names. The example above is for SecretsManager.AttachmentTargetType:

https://github.com/aws/aws-cdk/blob/ad326da3ae392b78dcfc349f246acdf3a389f283/packages/@aws-cdk/aws-secretsmanager/lib/secret.ts#L483-L527

Note that INSTANCE = 'AWS::RDS::DBInstance', and RDS_DB_INSTANCE = 'AWS::RDS::DBInstance', have different names, but the same values.

In the .jsii manifest generated for this module, the newer (non-deprecated) enums are not present:

Manifest:

    "@aws-cdk/aws-secretsmanager.AttachmentTargetType": {
      "assembly": "@aws-cdk/aws-secretsmanager",
      "docs": {
        "stability": "stable",
        "summary": "The type of service or database that's being associated with the secret."
      },
      "fqn": "@aws-cdk/aws-secretsmanager.AttachmentTargetType",
      "kind": "enum",
      "locationInModule": {
        "filename": "lib/secret.ts",
        "line": 483
      },
      "members": [
        {
          "docs": {
            "deprecated": "use RDS_DB_INSTANCE instead",
            "stability": "deprecated",
            "summary": "A database instance."
          },
          "name": "INSTANCE"
        },
        {
          "docs": {
            "deprecated": "use RDS_DB_CLUSTER instead",
            "stability": "deprecated",
            "summary": "A database cluster."
          },
          "name": "CLUSTER"
        },
        {
          "docs": {
            "stability": "stable",
            "summary": "AWS::RDS::DBProxy."
          },
          "name": "RDS_DB_PROXY"
        },
        {
          "docs": {
            "stability": "stable",
            "summary": "AWS::Redshift::Cluster."
          },
          "name": "REDSHIFT_CLUSTER"
        },
        {
          "docs": {
            "stability": "stable",
            "summary": "AWS::DocDB::DBInstance."
          },
          "name": "DOCDB_DB_INSTANCE"
        },
        {
          "docs": {
            "stability": "stable",
            "summary": "AWS::DocDB::DBCluster."
          },
          "name": "DOCDB_DB_CLUSTER"
        }
      ],
      "name": "AttachmentTargetType"
    },

@njlynch njlynch added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 9, 2021
@RomainMuller RomainMuller added effort/medium Medium work item – a couple days of effort module/compiler Issues affecting the JSII compiler p1 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 13, 2021
@RomainMuller RomainMuller removed their assignment Jun 24, 2021
@njlynch
Copy link
Contributor Author

njlynch commented Sep 9, 2021

Another example: aws/aws-cdk#16443

njlynch added a commit to aws/aws-cdk that referenced this issue Nov 26, 2021
There is an outstanding bug with `jsii`
(aws/jsii#2782) where duplicate enum values are not
available to the non-JS/TS languages. In these cases, only the first enum value
defined is present in the assembly.

For these cases, the first value was the deprecated one. This meant that only
the deprecated values were present, and once we stripped the deprecated
elements, we were left without values. For *v2 only*, we can reorder these enums
so the non-deprecated values appear in the assembly. Note that we *cannot*
backport this to `master` (v1), because it would introduce breaking changes to
our Java/Go/Dotnet/Python customers.
njlynch added a commit to aws/aws-cdk that referenced this issue Nov 26, 2021
There is an outstanding bug with `jsii`
(aws/jsii#2782) where duplicate enum values are not
available to the non-JS/TS languages. In these cases, only the first enum value
defined is present in the assembly.

For these cases, the first value was the deprecated one. This meant that only
the deprecated values were present, and once we stripped the deprecated
elements, we were left without values. For *v2 only*, we can reorder these enums
so the non-deprecated values appear in the assembly. Note that we *cannot*
backport this to `master` (v1), because it would introduce breaking changes to
our Java/Go/Dotnet/Python customers.
njlynch added a commit to aws/aws-cdk that referenced this issue Nov 26, 2021
There is an outstanding bug with `jsii`
(aws/jsii#2782) where duplicate enum values are not
available to the non-JS/TS languages. In these cases, only the first enum value
defined is present in the assembly.

For these cases, the first value was the deprecated one. This meant that only
the deprecated values were present, and once we stripped the deprecated
elements, we were left without values. For *v2 only*, we can reorder these enums
so the non-deprecated values appear in the assembly. Note that we *cannot*
backport this to `master` (v1), because it would introduce breaking changes to
our Java/Go/Dotnet/Python customers.
njlynch added a commit to aws/aws-cdk that referenced this issue Nov 26, 2021
* chore: fix build break caused by exported deprecated functions (#17719)

Functions (not member methods) aren't acknowledged by jsii, so they are not
stripped out of the compiled source, even when deprecated. These two functions
are deprecated and reference deprecated interfaces. When running the init
template tests against JS/TS, the tests fail with:

```
node_modules/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.d.ts(110,62): error TS2304: Cannot find name 'FixedResponse'.
node_modules/aws-cdk-lib/aws-elasticloadbalancingv2/lib/alb/application-listener-rule.d.ts(116,68): error TS2304: Cannot find name 'RedirectResponse'.
```

By -- on v2 only -- un-exporting the functions and inlining them in their
secondary usages, we can effectively "strip" these functions from the JS/TS.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

* chore: duplicate enum values not usable (#17731)

There is an outstanding bug with `jsii`
(aws/jsii#2782) where duplicate enum values are not
available to the non-JS/TS languages. In these cases, only the first enum value
defined is present in the assembly.

For these cases, the first value was the deprecated one. This meant that only
the deprecated values were present, and once we stripped the deprecated
elements, we were left without values. For *v2 only*, we can reorder these enums
so the non-deprecated values appear in the assembly. Note that we *cannot*
backport this to `master` (v1), because it would introduce breaking changes to
our Java/Go/Dotnet/Python customers.

* chore(release): 2.0.0-rc.33

Co-authored-by: Nick Lynch <nlynch@amazon.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: AWS CDK Team <aws-cdk@amazon.com>
rix0rrr added a commit that referenced this issue Nov 26, 2021
If two enum members have the same value, only the first one will
be retained.

This is a bit of an issue as we are renaming enum members: the named
version will never appear in the assembly, and so not work over jsii.

What's worse, if we *deprecate-and-strip* the original one, neither
of the enum members will appear.

Address this issue by working around TypeScript's natural tendencies
to collapse enum values. There was a bug in single-valued enum symbolIds
as well, which this PR also fixes.

Fixes #2782.
yuth added a commit to yuth/jsii that referenced this issue Mar 7, 2022
…alue

If two enum members have the same value, only the first one will
be retained.

This is a bit of an issue as we are renaming enum members: the named
version will never appear in the assembly, and so not work over jsii.

What's worse, if we deprecate-and-strip the original one, neither
of the enum members will appear.

Addressing the issue by failing the compilation by adding validation for
enum values

Fixes aws#2782.
@mergify mergify bot closed this as completed in #3412 Jun 29, 2022
mergify bot pushed a commit that referenced this issue Jun 29, 2022
…al (#3412)

If two enum members have the same value, only the first one will be retained.

This is a bit of an issue as we are renaming enum members: the named version will never appear in the assembly, and so not work over jsii.

What's worse, if we deprecate-and-strip the original one, neither of the enum members will appear.

Addressing the issue by failing the compilation by adding validation for enum values

Related change in CDK aws/aws-cdk#19320
Fixes #2782.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
@github-actions
Copy link
Contributor

⚠️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.

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. effort/medium Medium work item – a couple days of effort module/compiler Issues affecting the JSII compiler p1
Projects
None yet
2 participants