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.65k stars 245 forks source link

Allow Inline Disabling of JSII Errors and Warnings #3468

Open GEMISIS opened 2 years ago

GEMISIS commented 2 years ago

:rocket: Feature Request

Affected Languages

General Information

Description

JSII tries to enforce various coding standards as a part of its build process. This creates massive issues with various edge cases, as there is no way to disable these checks currently (even the "Diagnostics" section of the Documentation fails for most of its examples now). An example of this can be seen with the discord-bot-cdk-construct project, where Discord's APIs expects allowed_mentions (ie: camel case). However, because there is no way to disable JSII8002, it is impossible to build a TypeScript API that supports this today using JSII.

Proposed Solution

There needs to be a way to disable certain code sections inline, similar to what is in the above example for eslint.

peterwoodworth commented 2 years ago

If you have any ideas on how to implement this, we'd be happy to review a PR! Else, we may not be able to implement this for a while

RomainMuller commented 2 years ago

An example of this can be seen with the discord-bot-cdk-construct project, where Discord's APIs expects allowed_mentions (ie: camel case). However, because there is no way to disable JSII8002, it is impossible to build a TypeScript API that supports this today using JSII.

We made the decision to be opinionated here so that APIs have a consistent look-and-feel and we can reliably transpose these in other languages.

In the example provided, the way to go would be to expose an allowedMentions property "publicly", but serialize to allowed_mentions privately... Unless what you are trying to say here is that you would prefer to keep the original snake-casing in your exported API in order to meet user expectations?

alukach commented 1 year ago

We made the decision to be opinionated here so that APIs have a consistent look-and-feel and we can reliably transpose these in other languages.

@RomainMuller I appreciate the pursuit of uniformity, but I do agree that an escape hatch would be sensible in certain situations. For example, I am currently running into JSII8002 because I have a method on a construct to generate parameters for an rds.ParameterGroup. My understanding is that Postgres expects these values in snake case however to get around JSII8002 I suppose I have to do something like:

class MyConstruct extends Construct {
  constructor(scope: Construct, id: string) {
    super(scope, id);

    const params = this.buildParameters();
    const parameterGroup = new rds.ParameterGroup(this, "parameterGroup", {
      // ...
      parameters: {
        shared_buffers: params.sharedBuffers,
        effective_cache_size: params.effectiveCacheSize,
        work_mem: params.workMem,
        maintenance_work_mem: params.maintenanceWorkMem,
        max_locks_per_transaction: params.maxLocksPerTransaction,
        temp_buffers: params.tempBuffers,
        seq_page_cost: params.seqPageCost,
        random_page_cost: params.randomPageCost,
      },
    });
  }

  public buildParameters(): DefaultParameters {
    return {
      sharedBuffers: '...',
      effectiveCacheSize: '...',
      workMem: '...',
      maintenanceWorkMem: '...',
      maxLocksPerTransaction: '...',
      tempBuffers: '...',
      seqPageCost: '...',
      randomPageCost: '...',
    };
  }
}

export interface DefaultParameters {
  readonly sharedBuffers: string;
  readonly effectiveCacheSize: string;
  readonly workMem: string;
  readonly maintenanceWorkMem: string;
  readonly maxLocksPerTransaction: string;
  readonly tempBuffers: string;
  readonly seqPageCost: string;
  readonly randomPageCost: string;
}

This feels a bit cumbersome when I really just want:

class MyConstruct extends Construct {
  constructor(scope: Construct, id: string) {
    super(scope, id);

    const parameterGroup = new rds.ParameterGroup(this, "parameterGroup", {
      // ...
      parameters: this.buildParameters(),
    });
  }

  public buildParameters(): DefaultParameters {
    return {
      shared_buffers: '...',
      effective_cache_size: '...',
      work_mem: '...',
      maintenance_work_mem: '...',
      max_locks_per_transaction: '...',
      temp_buffers: '...',
      seq_page_cost: '...',
      random_page_cost: '...',
    };
  }
}

export interface DefaultParameters {
  readonly shared_buffers: string; // @jsii-ignore:JSII8002
  readonly effective_cache_size: string; // @jsii-ignore:JSII8002
  readonly work_mem: string; // @jsii-ignore:JSII8002
  readonly maintenance_work_mem: string; // @jsii-ignore:JSII8002
  readonly max_locks_per_transaction: string; // @jsii-ignore:JSII8002
  readonly temp_buffers: string; // @jsii-ignore:JSII8002
  readonly seq_page_cost: string; // @jsii-ignore:JSII8002
  readonly random_page_cost: string; // @jsii-ignore:JSII8002
}

If you have any ideas on how to implement this

With this said, I'm not familiar enough with JSII's internals to understand how to add such an escape-hatch comment.

RomainMuller commented 1 year ago

Implementing inline diagnostic moderation would likely require a significant re-architecting of the compiler infrastructure... I think it is the right thing to do in the long run, but it will be no small feat.