Graph V1 - All models are not in the same path

[rkodev]

All models are expected to be exported to the index file but some are not present
e.g

Expected import statement

import { RedirectRequestBody, RedirectRequestBuilderPostRequestConfiguration } from "./models";

VS : Current import statement

import { RedirectRequestBody, RedirectRequestBuilderPostRequestConfiguration } from "./communications/calls/item/redirect";

[rkodev]

cc @nikithauc

[baywet]

this is expected to avoid conflicts, you can have multiple endpoints ending with /redirect, that accept different things (so we can't reuse those models between endpoints).
The general rule of thumb is:

• inline models go next to their request builders
• component models go to the models directory/namespace

[nikithauc]

What is the difference between component and inline models?

We could have a different naming scheme for the inline models to distinguish on basis of their paths or endpoints.

For an SDK user and even the SDK gen user, not familiar or concerned with the API schema the perception would be all models are under the folder models based on the naming.

[nikithauc]

If we have a different naming scheme. We could have all models (inline and component) can be under the same folder called models.

Additionally we could generate a barrel exporting the API client and all the models from the same folder.

[baywet]

The new naming convention will need to be deterministic (to avoid shipping breaking changes from version to version) and avoid collisions across the whole API surface. But sure.

[nikithauc]

1. What would be the best naming scheme to resolve duplicate file names?

2. We could maintain a single file for models such as https://github.com/microsoftgraph/msgraph-sdk-typescript/blob/dev/src/me/outlook/supportedTimeZones/supportedTimeZonesResponse.ts and https://github.com/microsoftgraph/msgraph-sdk-typescript/blob/dev/src/users/item/outlook/supportedTimeZones/supportedTimeZonesResponse.ts

Is there a way to identity same models in different paths?

[nikithauc]

There are multiple models in different namespaces which have the same name. For example, models related to delta, translateExchangeIdsResponse.

• Duplicated Models such as translateExchangeIdsResponse, supportedTimeZonesResonse can be moved under the common/models folder
• Incase of model files with the same name but variations in the properties, example deltaResponse, the namespace index should export those using an alias. The naming convention of the alias will need to be deterministic.
• Incase of deltaResponse and similar cases we could make use of generics with something like value : T[].
• The generator will also have to generate an index file at the root exporting from all the sub-namespaces containing the models and enums.

[baywet]

Duplicated Models such as translateExchangeIdsResponse, supportedTimeZonesResonse can be moved under the common/models folder

No, they are not duplicated models. Keep in mind that Kiota doesn't know about OData, and even if it did assuming a response under one API path is the same as another one under another is just wrong. Even with OData semantics, that API path could have an annotation requiring a different type. If we start loosing precision now, we might corner ourselves in the future.

In case of deltaResponse and similar cases we could make use of generics with something like value : T[]

As long as it is supported by OpenAPI. However I don't believe it is right now.

[nikithauc]

cc: @sebastienlevert

[sebastienlevert]

I think the impact to the developers to fix this is huge. Right now, the developer gets this when using the SDK :

Which one do I choose? We should work on getting a seamless experience.

[baywet]

F12 on post would tell you which one.

[sebastienlevert]

Or even this one 😂

[rkodev]

It's a bit difficult to figure out as a dev which object to create, but with the interfaces its slightly easier without having to explicitly declare the type

const requestBody  = {
	description : "Update basic properties of application registrations",
	displayName : "Application Registration Support Administrator",
	rolePermissions : [
		{
			additionalData : {
				allowedResourceActions : [
					"microsoft.directory/applications/basic/read",
				],
			},
		},
	],
};

const result = async () => {
	await graphServiceClient.roleManagement.directory.roleDefinitionsById("unifiedRoleDefinition-id").patch(requestBody);
}

[sebastienlevert]

Agree, but as I don't have access to that version of the code as it's not yet released... I can't comment on this. Still, if I wanted a fully typed object I wouldn't know which interface to use...

[baywet]

F12 on patch?

[nikithauc]

The library should resolve that for the user. F12 would not serve as the solution considering multiple IDE's or OS such as windows, mac.

[sebastienlevert]

Using the smart help icon should be helpful. Here I don't know which one to get. I get it's automated and everything. But the developer experience should be our first priority and I don't think this is the case here.

Now, I'm using the non-interface version of the code... Maybe it fits more with the interface as I would not need to specify the classes up front and would using a simple json object... I am curious to see where this will lead us.