Revisit `as "id"` syntax / "as" keyword is confusing

[eladb]

We want to use as for casting, so let's revisit the way we specify an identifier for preflight objects.

One option is to replace the current usage with is:

new Bucket() is "myBucket";
new Bucket() is "yourBucket";

Another alternative is to add a synthetic keyword optional parameter called id to every preflight class initializer and plug it in:

new cloud.Bucket(); // defaults to "cloud.Bucket"

new cloud.Bucket(id: "my-bucket");

See below for some discussion.

Thoughts?

[skyrpex]

I kind of prefer the old fashioned way:

new Bucket(id: "yourBucket");

But I understand it leads to verbose and even possibly confusing paths...

So, is is fine to me.

[Chriscbr]

Sounds good to me, I think unlocking "as" for casting would be nice.

[yoav-steinberg]

What's the issue # for lang support for casting? Can help putting this in context.

I like new R() as "some_R" because it conveys the idea that there's a default and we're changing it. I like new R() id "some_R" because it's explicit about what we're doing, setting a unique id for the resource. I like new R() named "some_R" because it's very clear what we're doing, creating a resource and giving it a name. is feels a bit off. So not strongly opinionated.

[eladb]

Considering @skyrpex's idea. We can generate a synthetic optional keyword argument called id and then the usage will be:

new R(id: "some_R")

It's a bit of a hack, but from a DX perspective it feels right.

[Lancear]

What would this mean for struct expansion in function calls with an id field? Wouldn't this clash with the named arguments?

[marciocadev]

Why not use to for casting instead of as?

[eladb]

What would this mean for struct expansion in function calls with an id field? Wouldn't this clash with the named arguments?

Yes, it would clash. We can decide one of the following:

1. To make id a reserved name for keyword arguments (could be a bit annoying)
2. If there's an explicit id, just use this value as the logical ID. Basically "embrace the clash".
3. Use something like $id or some other more unique name for the synthetic identifier.

At any rate, I am growing to like this direction because it does result in a very intuitive DX.

new cloud.Bucket(id: "my-bucket");
new cloud.Queue();
new cloud.Function(id: "my-function", inflight () => {
  log("hey!");
});

I really hated the fact that the identifier was something you'd have to specify outside of the call.

[eladb]

Why not use to for casting instead of as?

as is commonly used for casting and I don't think we have a strong justification to shift the mental model here.

[Lancear]

Makes sense! Then I would go for embrace the clash. Seems like the best DX for when it clashes.

[marciocadev]

I like new R() id "some_R" because it's explicit about what we're doing, setting a unique id for the resource.

I think the most practical way would be to replace the current as with id, as suggested by Yoav's syntax.

[garysassano]

I personally find both idand is quite odd, since I'd expect an adverb in that position.

My proposals:

new R() alias "some_R"
new R() withId "some_R"
new R() withName "some_R"

[Chriscbr]

Considering @skyrpex's idea. We can generate a synthetic optional keyword argument called id and then the usage will be:

new R(id: "some_R")

It's a bit of a hack, but from a DX perspective it feels right.

Does the id always have to be the first argument?

[eladb]

Does the id always have to be the first argument?

No, it's a faux named keyword argument.

[Lancear]

When is the id named argument available? Always? I guess we don't only want to have it when the last parameter is a struct, but also allow it when the last argument is an optional or variadic?

[eladb]

Good point @lancear, it should be supported for in all initializer signatures.

From an implementation perspective, the compiler will not pass it through as a keyword argument to the function but will pass it as the 2nd construct argument.

So:

new cloud.Bucket(id: "bang", encrypted: true);

Will emit the following JavaScript:

new cloud.Bucket(this, "bang", { encrypted: true });

As I said, it's a hack.

@yoav-steinberg curious what you think about this direction?

[Chriscbr]

new cloud.Function(id: "my-function", inflight () => {
  log("hey!");
});

The parser right now expects all positional arguments to come before named arguments. To support usage like the one above, would we have to model this basically as a special case?

Can the "id" be added anywhere in the parameter list? ie

new cloud.Service(
  inflight () => { log("started!") },
  id: "my-service",
  inflight () => { log("done!"); },
  port: 8080,
);

[eladb]

The parser right now expects all positional arguments to come before named arguments.

Really? What's the motivation behind this restriction?

[Lancear]

The parser right now expects all positional arguments to come before named arguments.

I think this is great for readability, named arguments in between positional arguments can get quite difficult to read. Maybe we could ease the constraint to allow them either before or after all positional arguments, or only allow the named id argument also as the first argument.

[eladb]

I can see the argument for readability but maybe that's not something the language should be strict about, and leave it to the developer to decide, no?

[MarkMcCulloh]

When reading a function call, I generally expect the arguments to match the order that the function is expecting. Since struct expansion is just a sugar for struct construction as the last argument, seeing struct expansion tells me that I'm looking at the last argument.

Also worth noting we probably shouldn't call it "named arguments", as that confuses it with a similar concept in other languages that we don't support.

[Chriscbr]

Enforcing id can only be the first argument (if it's given) could be a reasonable compromise:

// Error: Expected "id" to be the first argument
new cloud.Function(inflight (x) => { ... }, id: "my-function",);

// Error: Expected "inflight (str): str" but got "str". Did you mean to write `id: "my-function"`? 
new cloud.Function("my-function", inflight (x) => { ... });

// OK
new cloud.Function(id: "my-function", inflight (x) => { ... });

[Lancear]

that would also make it clear it's not a normal struct expansion argument.

[Lancear]

btw is there any specific reason why we don't simply use the variable name of the resource as the id? Like that is the natural way to distinguish between instances of the same type in code already. 🤔

let myBucket = new cloud.Bucket();
new cloud.Queue(); // unnamed queue
let myFunction = new cloud.Function(inflight () => {
  log("hey!");
});

[eladb]

is there any specific reason why we don't simply use the variable name of the resource as the id

Variables are too volatile to serve as identifiers. These identifiers are mapped to physical resource names, and we don't want a simple rename of an identifier to have an impact on the application's infrastructure.

[eladb]

I am still not convinced that we need to be too strict about where keyword arguments are positioned in the function call. Requiring that id: is a special thing that must only be the first argument feels like a much higher cognitive load from a API consumer standpoint. From the consumer's perspective id: could just just be treated as another optional keyword argument.

I understand the stylistic argument (but that's not something we have to enforce), but I am not sure the fact that the struct is declared as the last argument justifies this strictness.

[hasanaburayyan]

So I personally am not strongly opinionated here but I do dislike this id argument direction, I kind of like the as "id" syntax as is.

Addressing some earlier discussions in the thread about not using as and maybe using named, alias, etc.. If this is for the sake of leaving as for casting I guess I see the point, however as would already be overloaded for not just casting but also aliasing.

bring "whatever" as lib;

So cognitively I dont think its too crazy to us new cloud.Bucket() as "my_bucket"

That said Id also be in favor of just changing new cloud.Bucket() as "my_bucket" to some thing along the lines of new cloud.Bucket() named "my_bucket"

[eladb]

I've been looking for an alternative for the "as ID" syntax not only to free up the "as" keyword but also because it's a cumbersome syntax which ends up appearing a lot in the language. The id: idea turns this into something much less messy in my opinion, which is why I am excited to explore it.

This is ugly from an aesthetic/stylistic point of view:

new cloud.Function(inflight () => {
  log("function 2");
}) as "f1";

new cloud.Function(inflight () => {
  log("function 2");
}) as "f2";

This is much better:

new cloud.Function(id: "f1", inflight () => {
  log("function 2");
});

new cloud.Function(id: "f2", inflight () => {
  log("function 2");
});

I also think it will reduce the cognitive load because it's one less concept of understand when you are creating objects.

[hasanaburayyan]

This is ugly from an aesthetic/stylistic point of view:

Yea but maybe its like a pug or chihuahua, soo ugly its cute 😁

[eladb]

Maybe.

[skorfmann]

A bit torn between as (or similar named keywords) and something like a virtual argument. While I found the as "myid" syntax odd in the beginning, it's nicely conveying that it's not a functional aspect of the construct, but something which serves a special purpose. Also, it's easy to add / remove on existing constructs.

On the other hand, having actual arguments probably fits better in the mental model of how to create and name constructs (in particular people coming from a cdk background).

Speaking of cdk, how would this work for cdk dependencies (e.g. resources from the @cdktf/aws-provider).

[eladb]

The proposed id: syntax is just sugar for what we have today. So basically when using CDK constructs, the id: keyword argument will be emitted as the 2nd positional argument of the construct.

[eladb]

After multiple discussions with multiple people, I am still leaning towards the synthesis id: keyword. I think it will give us the best result from a DX perspective, which is king...

Together with this change, I want to also allow to optionally call super(id: "custom id") from constructors of classes that don't have a base class, in order to allow classes to customize their identity:

struct UserProps {
  name: str;
  last: str;
}

class User {
  new(props: ...UserProps) {
    super("${props.name}-${props.last}");

    // cool!?
  }
}

let user = new User(name: "elad", last: "ben-israel");
assert(user.node.id == "eladb-ben-israel");

I also think we should implement #4846 (explicit struct expansion) and #4812 (allow keyword arguments anywhere) to complement this change.

[yoav-steinberg]

Together with this change, I want to also allow to optionally call super(id: "custom id") from constructors of classes that don't have a base class, in order to allow classes to customize their identity

I think that if we allow this then the id: named arg needs to be mandatory:

class User {
  new(props: ...UserProps) {
    super(id: "${props.name}-${props.last}"); // Calling an implicit super ctor requires an explicit `id:` named arg
    super(); // This is a no-op, but compiles just because that's what you'd expect
    super(scope: some_other_scope); // We might want to support this too?
  }
}

Also what about the case where we want to use the auto-generated id in our class:

class User {
  new(props: ...UserProps) {
    super(id: "${this.node.id}-${props.name}-${props.last}"); // Override the auto generated ID somehow??
  }
}

[eladb]

named arg needs to be mandatory

Not sure I understand why you are saying it needs to be mandatory.

Can you elaborate?

Also what about the case where we want to use the auto-generated id

Maybe simply props.id?

---

I am aware that this direction might be too hacky. Let's analyze it a little further and see if we can achieve nirvana. Otherwise we can continue our exploration.

[github-actions[bot]]

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

[staycoolcall911]

@ShaiBer proposed the following syntax for class members in #5941:

bring cloud;

class A {
  b1: cloud.Bucket as "B1";
  b2: cloud.Bucket as "B2";

  new () {
    this.b1 = new cloud.Bucket();
    this.b2 = new cloud.Bucket();
  }
}

new A();

[Chriscbr]

What would this mean for struct expansion in function calls with an id field? Wouldn't this clash with the named arguments?

Yes, it would clash. We can decide one of the following:

1. To make id a reserved name for keyword arguments (could be a bit annoying)
2. If there's an explicit id, just use this value as the logical ID. Basically "embrace the clash".
3. Use something like $id or some other more unique name for the synthetic identifier.

Here's a concrete example of an edge case we might need to address. In Wing you can import any number of CDKTF libraries, including a library for Docker which has a class representing a Docker Swarm config object: https://github.com/cdktf/cdktf-provider-docker/blob/071f68f14adf315d342d628c54e880f8e5cea2ec/src/config/index.ts#L27. Note that one of the named arguments for the class (as described by ConfigConfig) is named "id". I don't believe this argument is meant to be set under normal usage, but it's part of the public API, so a user should be able to set it if they want.

bring "@cdktf/provider-docker" as docker;

// does "id" refer to the docker config ID? or the construct ID?
new docker.Config(name: "foo_config", data: "fake_data", id: "MyConfig");

[eladb]

I think we could decide that id is reserved and when we import JSII types that have an id prop, we mangle to id_. Not ideal, but will likely be okay.

We can also decide the term will be something a bit less common like:

• ident
• oid (object id)
• nodeid
• nid
• wid
• cid

(And still mangle imports but that's going to be much less common)

[Chriscbr]

I think using a reserved identifier as a special keyword argument feels like the best solution so far. I'd propose using @id since we have already begin using the @ symbol to be indicative of other kinds of special language capabilities like macros.

new cloud.Bucket(@id: "MyBucket", public: true);

To someone new to the language, the syntax makes it relatively clear that IDs are a special thing since it has this @ character, but it's also clear from its position that it's an argument to the bucket. If you try reordering the arguments (like putting it after public above), it will also work as you'd expect. And lastly, any existing code that uses "id" as a field in a struct will continue to work as expected since those fields won't have the @ character in front.

[eladb]

I think we might have nailed it!