typescript icon indicating copy to clipboard operation
typescript copied to clipboard

Support for variable resolution pattern from Serverless

Open ryansonshine opened this issue 4 years ago • 6 comments

Use case description

Region is defined as a union of string literals which works fine if the region is being implicitly set, however when attempting to use Serverless variables a type error is raised.

The following is valid:

const serverlessConfiguration: AWS = {
  provider: {
    name: "aws",
    runtime: "nodejs12.x",
    region: "us-east-1",
  },
}

The following is considered invalid:

const serverlessConfiguration: AWS = {
  provider: {
    name: "aws",
    runtime: "nodejs12.x",
    region: "${opt:region, 'us-east-1'}",
  },
}

Proposed Solution

Add string type to the end of the union type for region to address this issue. The tradeoff being we lose type safety but keep auto completion and fix the issue of variables being used for region. I'm open to any/all other proposed solutions if anyone has any ideas.

ryansonshine avatar Dec 16 '20 05:12 ryansonshine

Hi @ryansonshine, thanks for reporting this issue.

Your solution is fine with me. This should be done in serverless/typescript and not in serverless/serverless since the framework will actually resolve "${opt:region, 'us-east-1'}" before proceeding with JSON-schema validation.

The definitions of this repository are programmatically generated from JSON-schema definitions contained in serverless/serverless. In order to make this modification - which does not only affects the region keyword, but all enums - we should implement a custom post-compiler to add the last union with a string type.

At the moment, json-schema-to-typescript does not offer such option. I'm working on switching to quicktype - see #1 - and should have it implemented this week. Quicktype offers much wider configuration option, with the ability to implement custom converters. One of those could be parsing JSON-schema enums containing only strings to add this last union. WDYT ?

fredericbarthelet avatar Dec 16 '20 08:12 fredericbarthelet

Sounds good to me, subscribing to #1 to track progress. I'll implement the converter to cover this bug after the switch is made.

Thanks!

ryansonshine avatar Dec 16 '20 11:12 ryansonshine

any update when this will be resolved?

pjain11 avatar Apr 22 '21 20:04 pjain11

@pjain11, this issue is blocked by #1 . It will be resolved shortly after

ryansonshine avatar Apr 23 '21 00:04 ryansonshine

casting the command line argument to AWS["provider"]["region"] helped to fix this issue for me. region: "${opt:region, 'us-east-1'}" as AWS["provider"]["region"],

ganimp84 avatar Oct 14 '22 03:10 ganimp84

Thx you, @ganimp84

Abdalodj avatar Feb 23 '23 10:02 Abdalodj