Coding standards around type annotations in function signatures

[danmuzyka]

Let's come up with a guideline around how to write type annotations in our function signatures. Here are a few examples of how it could be done:

  protected _conditionalSlotTemplate({
    elementName,
    slotNameStub,
    wrapperElementType = 'div',
    ariaLabel = null,
  }: {
    elementName: string;
    slotNameStub: string;
    wrapperElementType?: string;
    ariaLabel?: string | null;
  }): TemplateResult | null {

type ConditionalSlotArgs = {
    elementName: string;
    slotNameStub: string;
    wrapperElementType?: string;
    ariaLabel?: string | null;
  };

// ...

  protected _conditionalSlotTemplate({
    elementName,
    slotNameStub,
    wrapperElementType = 'div',
    ariaLabel = null,
  }: ConditionalSlotArgs): TemplateResult | null {

interface ConditionalSlotArgs {
    elementName: string;
    slotNameStub: string;
    wrapperElementType?: string;
    ariaLabel?: string | null;
  }

// ...

  protected _conditionalSlotTemplate({
    elementName,
    slotNameStub,
    wrapperElementType = 'div',
    ariaLabel = null,
  }: ConditionalSlotArgs): TemplateResult | null {

What do we want to make our standard? There was some discussion recently that if we define our type separately from the function signature, we could export the type itself from the file, to make available to other files. I also think that declaring an interface or a type alias separately from the function signature can improve readability as well.

[damontgomery]

Thanks, Dan.

I think the first is most readable since the types are within the function definition.

I realize that differs from your point of view. Having to scroll means keeping the context in my head rather than seeing it in one place.

Do we have examples of re-using these types of object argument types in multiple places?

I can't think of any off the top of my head and that would help me understand the value of separating it.

Is there a strong use case for type over interface?

See Typescript recommendation to prefer interfaces: https://www.typescriptlang.org/play#example/types-vs-interfaces. They say interfaces have better error messages and types can be imported using import type instead of import.

Is there a separate benefit to building an interface for the whole class including method types?

I have been able to re-use those in testing and that has been helpful. I don't actually find the automated tests useful and would be interested to see useful examples.

See https://github.com/phase2/outline/blob/next/src/components/base/outline-modal/test/outline-modal.test.ts and https://github.com/phase2/outline/blob/next/src/components/base/outline-modal/outline-modal.ts.

That also seems like a decent way to summarize the shape of the class in one place, all the properties and methods. I would still prefer to group the logic together and the type / interface definitions at the top break that pattern, but if the trade off is less copy & paste across files, that works OK for me and it's what I've done.

Final thoughts

I would be OK adopting any of these patterns, but feel like we should provide real world examples of why to prefer one over the other before deciding. Those real world examples could include who finds what more readable.

Thanks again.

[grshane]

My preference is to use Interfaces and to have them in a separate declaration like in the 4th code snippet in @danmuzyka's message. Having the interface away from the main functionality allows me to separate inputs, outputs, and the functionality of what the file is doing. In the case of utilities or helper functions, it is incredibly useful to be able to import the types into their usages

[danmuzyka]

It sounds as though @damontgomery and @grshane both prefer interfaces over type aliases for this use case (if the choice were only between those two options), if I am understanding your arguments correctly. I am also inclined to pick interfaces over type aliases for the reason that Dan mentioned (the official recommendation from the Typescript documentation).

I think the points Dan made about creating an interface for the entire class, rather than separate interfaces for individual return values or parameters, makes sense in cases where those data types are only used in that one place.

I am also happy to do as Dan suggests and defer to the opinions of people who have used Outline more extensively on real world projects, and what they think is most useful, based on their experience using it.

[danmuzyka]

Also, if Lit2 has any built-in opinions about which pattern to use, or if a majority of projects that use Lit2 follow a particular pattern, I think that should weigh heavily on our decision.

[himerus]

I am also happy to do as Dan suggests and defer to the opinions of people who have used Outline more extensively on real world projects, and what they think is most useful, based on their experience using it.

@danmuzyka I would argue here that this is a topic outside of anything actually related to Outline, so having worked with Outline plays little into having a valid opinion on this matter. This is a much more generic coding standards question IMO.

If it's a one off that's never reused outside of it's context, then do whatever, but I am of the opinion for readability it should be all at the top of the file and setup to be made available as needed to other components/scripts/stories/tests.

We definitely I'm sure could draw inspiration from Shoelace. Most of what's there has been pure gold so far. There are definitely other places we could draw some of those ideas from.

[danmuzyka]

Hey @himerus , thanks for articulating that perspective. You make a valid point, and I've changed my mind on this matter.

I wrote that portion of my comment out of concern that my own ideas might be based too much on my own armchair philosophizing about what our coding standards should be, and that other people who have used Outline more extensively than I have would recognize flaws in my thinking that would have been more obvious to me after spending more time using the Lit2 version of Outline. I was worried about my own opinions being less informed than those of others, and I didn't want to overshadow other opinions.

However, as I think about it more, I realize that the conclusion I drew was far too broad, and lack of direct experience on client projects should not detract from the value of someone's perspective. Thank you for bringing this to my attention.