Support for the unknown type

[dgreene1]

This is a proposal for the unknown type.

Before anyone takes on this PR, we need to

• allow the object literal PR and the type alias PR to close
• we need to determine what input and expected outcomes should be for aggregates and unions that contain unknown

Value that the unknown type offers:

Sometimes an API might allow the consumer to pass arbitrary data down to the service that will then be persisted in a schema-less DB like Mongo or PostGres’s JSONB. So as where the any type has risks associate, the unknown type more clearly communicates “I wouldn’t recommend trying to unwrap or utilize any subset of this data since it could have any shape or type.”

[dgreene1]

I’m creating this thread @WoH / @HoldYourWaffle due to the excellent guidance from the ElasticSearch contribution guidelines.

If you have a bugfix or new feature that you would like to contribute to Elasticsearch, please find or open an issue about it first. Talk about what you would like to do. It may be that somebody is already working on it, or that there are particular issues that you should know about before implementing the change.

We enjoy working with contributors to get their code accepted. There are many approaches to fixing a problem and it is important to find the best approach before writing too much code.

[WoH]

What is the expected representation of the unknown type on its own?
Imagine a controller fn returning unknown. What's that intended to be documented as?

[dgreene1]

That’s an excellent question @WoH

What is the expected representation of the unknown type on its own?
Imagine a controller fn returning unknown. What's that intended to be documented as?

I agree that it would be strange to have an API return unknown. But maybe we already support returning any which is essentially the same.

I’ll update the top of the issue to explain the context and value of the unknown type:

Sometimes an API might allow the consumer to pass arbitrary data down to the service that will then be persisted in a schema-less DB like Mongo or PostGres’s JSONB. So as where the any type has risks associate, the unknown type more clearly communicates “I wouldn’t recommend trying to unwrap or utilize any subset of this data since it could have any shape or type.”

So I see it being most value with the @Body() decorator.

[WoH]

Basically binary data? From the OpenAPI3 Spec:

requestBody:
  content:
    application/octet-stream:
      # any media type is accepted, functionally equivalent to `*/*`
      schema:
        # a binary file of any type
        type: string
        format: binary

[WoH]

But maybe we already support returning any which is essentially the same.

We document any like an object with unspecified properties, which is IMHO wrong and maybe should not be repeated.
On the validation side: We just don't do any checks, which is the most reasonable behavior for any.
unknown should check for existence at least I think.

the unknown type more clearly communicates “I wouldn’t recommend trying to unwrap or utilize any subset of this data since it could have any shape or type.”

Isn't unknown supposed to help TS help you be careful before 'utilizing any subset of the data' which any did not?

[dgreene1]

We document any like an object with unspecified properties, which is IMHO wrong and maybe should not be repeated.

No it’s not a mistake, it’s actually how swagger and OpenAPI 3 document the any type. See here: https://swagger.io/docs/specification/data-models/data-types/#any

On the validation side: We just don't do any checks, which is the most reasonable behavior for any.
unknown should check for existence at least I think.

Since an unknown value can also be undefined or null in TypeScript, we can’t validate for existence. So unknown would have all of the same validation qualities (or lack-thereof) as any.

[HoldYourWaffle]

I’m creating this thread @WoH / @HoldYourWaffle due to the excellent guidance from the ElasticSearch contribution guidelines.

Do I sense some shade 😉
Just to be clear, I agree that the guidelines you cited make sense for this project.

we need to determine what input and expected outcomes should be for aggregates and unions that contain unknown

As an actual "input" to the discussion: what do you mean by aggregates in this context?

[dgreene1]

Do I sense some shade

No. Please assume best intentions. There is literally no shade or hard feelings. In fact, I care deeply about the tsoa community and the volunteers that work on it. Since your PR pointed out valuable improvements to the type resolver, I took the time to think of a feature enhancement that would give us a proper reason to refactor the type resolver code.

I would prefer to stop talking about the philosophical aspects of refactoring. My main point here is (as it has always been): I can not spend time reviewing a PR that does not add functionality to the user (which Luke agrees on) or that fixes an issue that has been created and verified.

I don't want us arguing over things that don't result in some benefit for the end-user.
~ LukeAutry (#445 (comment))

My availability to review PRs, merge PRs, and to publish new version of tsoa is very limited. I want to be very clear that conversations like the ones in the #445 PR take time away that I could be using to make tsoa a better library. That's why we will soon have "rules of the game" as part of our contribution guidelines. The rules I've set forth so far (i.e. having a real issue number that has been marked as "Help Wanted", having unit tests, etc.) are the contributor guidelines as long as I am the maintainer.

In summary, I created this issue for you so that you would have mechanism by which to contribute within the guidelines. I'm doing this so that your contributions can be accepted and so that my time can be used efficiently.

As an actual "input" to the discussion: what do you mean by aggregates in this context?

I'm referring to the & type operator. Check out this link for all of the possible aggregates and unions for unknown and how it's supposed to get resolved: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-0.html#new-unknown-top-type

For convenience here's an example:

type T22<T> = T & unknown;  // T
type T23<T> = T | unknown;  // unknown

[WoH]

No it’s not a mistake, it’s actually how swagger and OpenAPI 3 document the any type. See here: https://swagger.io/docs/specification/data-models/data-types/#any

I did not know that and it's really valuable information, thank's a lot for pointing it out.
I retract my initial assessment.

Since an unknown value can also be undefined or null in TypeScript, we can’t validate for existence. So unknown would have all of the same validation qualities (or lack-thereof) as any.

I should have clarified I did not mean JS existence. But since unknown may be undefined I'd agree.

[HoldYourWaffle]

I would prefer to stop talking about the philosophical aspects of refactoring.

As time consuming as it may have been (and it takes a lot of time for me to write my responses in somewhat OK-English), I did find it very insightful and I'm happy I got to see another point of view (no matter how much I disagree with some parts of it 😉)

My main point here is (as it has always been): I can not spend time reviewing a PR that does not add functionality to the user (which Luke agrees on) or that fixes an issue that has been created and verified.

I definitely agree that the original form of "that PR" was waaay too much effort to review, I honestly didn't even think of that. However, I can assure you that the issues+PR I'm going to create for my separated and atomic cleanups will be very easy to review. I'll make sure that everything has its own issue with reasoning and relevant diff information so you (or luke, he should probably get a say in this too) can simply click a link, look, switch tabs and say "yes" or "no", it'll probably take about 5 minutes per change including writing a response. Afterwards I'll make sure that everything is ready for merge.

I hope that in this way we can make the type resolver more readable and accessible without a time consuming review process. I'm not trying to "push my opinion" here, I honestly believe that this cleanup is needed (at least some parts of it) and will provide long-term benefits. It feels to me like you're marking it as "not useful" or "not worth reviewing" upfront, but you haven't actually seen any of it yet. I only want it to at least get a fair review.

Going back on-topic to the unknown type: I'll properly read myself into the material tomorrow (if I remember...).

[WoH]

we need to determine what input and expected outcomes should be for aggregates and unions that contain unknown

I'm referring to the & type operator. Check out this link for all of the possible aggregates and unions for unknown and how it's supposed to get resolved: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-0.html#new-unknown-top-type

For convenience here's an example:

type T22<T> = T & unknown;  // T
type T23<T> = T | unknown;  // unknown

Current union/intersection should take care of that, making it (presumably) a lot easier to implement. Just add a validation for unknown that accepts everything (based on a metadata and spec representation for the (primitive) type).

Current thoughts bringing type aliases into play here:
The type aliases are easy with my suggested patch in mind, otherwise, we'd create a ref type every time T22 gets used with a concrete typeArgument. (T22User, T22Response, ...). Current alias handling also should do most of the heavy lifting, it's just another if nested in a lot of other ifs.

[HoldYourWaffle]

it's just another if nested in a lot of other ifs.

Sounds lovely 😉

[dgreene1]

I'll remove the "feedback" label and add the "help wanted" label now that we've decided that:

• unknown will have runtime validation and swagger generation that is equivalent to the any type
• unknown | T will always be unknown`
• unknown & T will have to behave like the TypeScript readme explanation (https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-0.html#new-unknown-top-type)

[HoldYourWaffle]

@dgreene1 Can you please assign this to me so I don't forget?

[dgreene1]

Can you please assign this to me so I don't forget?

@HoldYourWaffle I don’t feel comfortable with the idea of reserving work. If someone else is able to provide a PR for this issue before you do and it meets all of the acceptance criteria, then that’s the one that we will accept. If we reserve this for you and you forget to do it, then all future contributors will ignore this PR and it won’t get done. If you you want a reminder, please use a personal reminder app like iOS’s Reminders, etc.

[HoldYourWaffle]

The main reason I asked to be assigned is to avoid double work, the reason why you want these issues in the first place (which is a very good reason don't get me wrong).
The second reason, and the one I listed, is that I can see all my assigned issues/PR's in one convenient list. This allows me stay organized when I'm contributing to 3 upstream projects for another project on a somewhat tight deadline. I could indeed use a different "reminding" app, but that has some big drawbacks compared to just "being assigned" like in every other issue I'm currently working on.

There is of course a risk that I won't get this done in a timely matter, I actually wouldn't be surprised given my aforementioned schedule. If that were to happen, you could just post a comment asking for a sign of life or even deassign me if I still don't respond. Stale bot can probably be configured to handle this automatically if it doesn't do this already.

It's true that an issue being assigned can deteriorate contributors from working on it, which is kind of the idea in order to prevent double work. However, I feel like there are other, more important things in this project that deteriorates contributors, my most recent experience being that my comment on #458 was completely ignored.

Since I've started contributing to this project I've felt hindered or unwanted almost every single step of the way. I understand what you're trying to do and I definitely understand that not everything can or should be added/implemented/... for various reasons. Not everything I've done was good of course (cough #445), but when you get this feeling even for trivial things like #447, #360, #365, #451 (#242) things stop being 'fun' really quickly, even when assuming best intentions. Maybe it's the language barrier or maybe it's just me being stupid or annoying, either way it makes me not want to contribute anymore.

Because I depend on this project in production (with a use-case that requires some extra "help") and because I think it's childish to avoid improving something because you've had bad experiences with it in the past, I'll still contribute any improvements that I deem necessary for my use-case (which is currently mostly #451, #365 and a pending type resolver cleanup [this issue]). However, at least for now, the will to contribute to this project "for fun", e.g. just to make it better for others, is completely gone.

[dgreene1]

I’m sorry that you feel that way. I really am.

If you would like to submit code to help tsoa, that would be sincerely appreciated.

[dgreene1]

Since I was very saddened by your message above, I wanted to take time to reflect on your feedback.

@HoldYourWaffle here's a record of where all of the issues you presented are currently. I believe you'll see that I have done my best to be open minded. In all of the cases I have either:

• changed my mind to accept your suggestion
• offered a compromise

CLI always requires a valid config for both swagger & routes #447

• @HoldYourWaffle's original request: users don't have to provide both config objects if they want to do one of the two actions (route generation vs swagger generation)
• My original stance: I agreed instantly, I just didn't know how to help
• My attempt to help:
  • I offered a solution that would do exactly what you're asking

Contributor guidelines #458

• @HoldYourWaffle's original request: a conversation about how refactoring PRs would be accepted
• My original stance: That was a great idea to have contributor guidelines and to have a conversation about them. As far as your comment (updates language to be more open per #457 #458 (comment))... I simply didn't see it until your pointed it out now. I'm sorry. If you have more to say about the text in the contributor guidlines, I have purposefully kept the contributor guidelines thread issue open so that people can have an open dialog about these types of issues. This should stand as a good will gesture of the "open mindedness" you have been asking me to have (and that I believe have had).
• My revised stance after I listened to you:
  • I am now willing to accept that refactoring can happen as a PR without fixing an issue or adding a feature. Note: this is a *major reversal of my original opinions and should illustrate how I'm willing to not just listen... but to change my mind.
  • Speaking of which, if you would like to submit your refactoring of the TypeResolver, feel free to do so. Just please use the tagged union approach that Luke recommended since that should allow us to delete the as keywords that you wisely pointed out were bad practice.

Add option to change output filename #360

• @HoldYourWaffle's original request: allow a user to change routes.ts to a customizable name
• My original stance: I was curious about the value of the change. I never disagreed, I just asked for more information.
• My attempt to help:
  • I listened to your reasoning. I then agreed. I then worked with you to fix the PR so that the feature would not change existing behavior. I then approved the PR

Allow aliased imports for parameter decorators #365

• @HoldYourWaffle asked for something that I did not understand at first (I accept responsibility for being dense/ uninformed about passport.js)
• My attempt to help:
  • I asked for clarification on the goal, I listened, and then I offered how I believe it is possible to accomplish your goals currently without any PRs to tsoa.

Tuple support #451

• @HoldYourWaffle asked for a feature that swagger/openAPI does not suppoort
• My original stance: we wouldn't support tuples
• My revised stance after I listened to you:
  • I listened, I then proposed a compromise and specific steps on how the goal could be achieved. Also didn't close the door for full tuple support, I simply said that it should wait until swagger/OpenAPI supports it. I recognize that unlike my above interactions, this was rather "hard-line" thinking, but I think the compromise shows at least my efforts to listen to your desires.

Summary:

I've done my best to listen to everything you have said (including your complaints about yarn and tab spaces). In all of the above cases, you have made an impact on my thought process. I have listened and you have changed my mind multiple times. And your features are all actively being considered. So for you to say you are "unwelcomed" is not true. You and your contributions are very welcome as illustrated by the efforts I have gone to understand your perspective.

As far as your concern for being "hindered," I have done my best to create an objective environment where everyone is treated fairly and equally by the same rigorous quality standards that a library of tsoa's popularity deserves. Quality gates can often be seen as hindering; however, I have also approved more tsoa PRs in the last month than have every been approved for tsoa. What I'm saying is that many contributors are finding tsoa to be a place where features can be added in an an "un-hindered" manner.

I like to think that the increase in contributions and releases is due to the support and care I am putting into maintaining the tsoa community. I hope you can find a way to understand my sincere interest in making this an enjoyable and open place to contribute.

[HoldYourWaffle]

First off, I want to apologize for the very late response, as I said before I'm currently working on a project with a somewhat tight deadline. I think the notifications for your comments didn't come through correctly, since I didn't notice them until stale bot commented on #446. I'll be responding to all your comments (at least those I could find) shortly.

Next up, I'm really, terribly sorry I saddened you, this was nowhere near my intention and I hope you can forgive me for this. My comment was written in emotions of frustration and doubt, feeling like I'm not able to express myself correctly in my non-native language.
Reading your comment (and the other ones I missed) I get the feeling that there's a lot of miscommunication happening. Please keep in mind that English is not my native language, and please tell me if I'm not explaining myself well enough.

I'll be responding to the rest of your comment tomorrow since it's very late and tired. I can say from experience that writing these sort of things doesn't go very well when I'm tired ;)

[HoldYourWaffle]

I want to start by responding to the end of your comment:

I like to think that the increase in contributions and releases is due to the support and care I am putting into maintaining the tsoa community. I hope you can find a way to understand my sincere interest in making this an enjoyable and open place to contribute.

I couldn't agree more, and I deeply appreciate and admire your interest and ability to listen and sometimes even change your mind after discussion on these important topics.

On to the actual "material" here. Perhaps 'hindered' was not the right word to use, a certain kind of "resistance" would probably be a better description. I won't be going into detail about why I felt 'unwelcome' for each individual issue (unless you really want me to) because I don't think this matters anymore. I have already made up my mind about contributing to this project, and this probably won't change anytime soon. Please don't think this is "your fault" or anything like that, I have many reasons (some of which I haven't mentioned), most notably this next one:

I think my vision on what this project is or should be is just too different from yours (most likely because I use it in a different way). I'd love to have an open discussion about this sometime (perhaps not in a github issue) since I'm interested in how other people use this amazing library.
You're the maintainer of course, so in the end you're the "benevolent dictator" as Luke put it nicely, the one that decides what this project will, should and is going to be.

I've been thinking about creating a fork of this project that's more aligned with my use-case. (This is why all my PR's are coming from HoldYourWaffle/tsoa-iso.) I don't like forking projects since I believe in collaboration (I think everyone does), but I feel like our goals are just too far apart.
Maybe it's a good idea to have the open discussion I mentioned earlier some time, maybe we can come to a compromise that makes each of us happy.