Adding models + interfaces #675
Conversation
| - [NodeJS e2e using the Service library](#node-service-library) | ||
| - [NodeJS e2e using the Core library](#node-core-library) | ||
|
|
||
| ## <a id="foundation"></a> Foundation |
There was a problem hiding this comment.
| ## <a id="foundation"></a> Foundation | |
| ## Foundation |
github adds the anchors automatically for you when using headings
There was a problem hiding this comment.
They don't allow me to "know" what will be the generated anchor. So I prefer this for more complex TOCs.
There was a problem hiding this comment.
it uses kebab casing Foundation => foundation, My Foundation => my-foundation
| ```typescript | ||
| export class SendMailRequestBody implements Parsable, SendMailRequestBody { | ||
| /** Stores additional data not described in the OpenAPI description found when deserializing. Can be used for serialization as well. */ | ||
| private _additionalData: Map<string, unknown>; |
There was a problem hiding this comment.
update additionalData to Record @nikithauc is already working on that change
There was a problem hiding this comment.
It's just there are an "example". Didn't mean to replicate the actual implementation.
There was a problem hiding this comment.
then add a comment saying the purpose of this snippet is to demonstrate the api surface, not the implementation details. I don't want people to mistake that for a technical spec
| Thanks to the TypeScript nature of allowing merging of Classes and Interfaces with the same name, models are represented in a way that allows both rich capabilities of the classes (backing stores, dirty tracking, etc.) and simplicity of using interfaces. | ||
|
|
||
| ```typescript | ||
| export class SendMailRequestBody implements Parsable, SendMailRequestBody { |
There was a problem hiding this comment.
maybe add a comment mentioning that interfaces and classes might have a different name or live in different packages depending on implementation/runtime constraints
| */ | ||
| public constructor(value?: Partial<SendMailRequestBody>) { | ||
| if(value) { | ||
| Object.assign(this, value); |
There was a problem hiding this comment.
this section might be a bit more complex than that, we need to account for properties that are complex types (message body) or collections of complex types (message to recipients) as well.
There was a problem hiding this comment.
Not the goal. I Think I'll simply remove the code. This is not meant for the internal implementation, but the way developers use the SDK.
There was a problem hiding this comment.
yep, replace by a comment saying the actual implementation object gets instantiated from the partial values
|
This PR should be in somewhere Kiota because this is the design applicable to the generation process. |
|
|
||
| The models should be easily consumable by developers in a natural format. For JavaScript and TypeScript developers, interfaces are the most common way to understand, represent and send data to endpoints. | ||
|
|
||
| Thanks to the TypeScript nature of allowing merging of Classes and Interfaces with the same name, models are represented in a way that allows both rich capabilities of the classes (backing stores, dirty tracking, etc.) and simplicity of using interfaces. |
There was a problem hiding this comment.
Importing a class and interface with the same name gives me this issue.
Try the following:
//File a.ts
export interface name{
id:string;
test:string;
}
export class name{
id:string;
type:string;
test:string;
public getFieldDeserializers():Array<string>{
return ["name","type"];
}
}
//b.ts
import {name} from "./a";
const test: name ={ id:"name", type:"type"};
I get a prompt on the test variable that Error: Type '{ id: string; type: string; }' is missing the following properties from type 'name': test, getFieldDeserializersts(2739).
Interfaces can have duplicate names and that works fine in TypeScript because they do not transpile. However, we are not getting rid of the problem that we see today that requires all properties to be set when using a json initialization pattern because the class requires all the properties.
There was a problem hiding this comment.
We would need to think about the import pattern when using same named interfaces and classes.
No description provided.