Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

investigate Selective imports for static functions to reduce build time #106

Closed
Tracked by #1
maisarissi opened this issue Aug 4, 2022 · 2 comments
Closed
Tracked by #1
Assignees
Labels
enhancement New feature or request

Comments

@maisarissi
Copy link

maisarissi commented Aug 4, 2022

The navigation properties as currently declared on the request builder/api client directly. While this is convenient for discovery, it also means the whole graph structure gets pulled as soon as the client is used, which for such a large API means a long compilation time, negative impacts to auto completion etc...

Since those functions are "declared statically" (go idioms) we could move their declaration to the sub module they point to.
I.E. the declaration for users() on the service client would live under the users submodule and not on the root module.

What that means is that people would need an additional import (msgraphsdkgo/users) before they can use the navigation property, but it should greatly reduce build times since the compiler would only be pulling things the application needs to begin with.

We might run into circular dependencies while experimenting with that.

@maisarissi maisarissi added the enhancement New feature or request label Aug 4, 2022
@baywet baywet changed the title Selective imports for static functions investigate Selective imports for static functions to reduce build time Aug 5, 2022
@baywet baywet mentioned this issue Aug 5, 2022
25 tasks
@baywet
Copy link
Member

baywet commented Nov 4, 2022

I took some time to fiddle with that. Go doesn't support defining function receivers outside of the package where the type is defined.
What that means is we cannot have the "me" function defined in the "me" package instead of in the root one.
The alternative would be to have composed types defined at each package level. which would like to an experience like this one.

Current

result, err := client.Me().Drive().Get(context.Background(), nil)
if err != nil {
    fmt.Printf("Error getting the drive: %v\n", err)
    printOdataError(err)
}
fmt.Printf("Found Drive : %v\n", *result.GetId())

Workaround

//medrive is an import of github.com/microsoftgraph/msgraph-sdk-go/me/drive
result, err := medrive.Client(client).Get(context.Background(), nil)
if err != nil {
    fmt.Printf("Error getting the drive: %v\n", err)
    printOdataError(err)
}
fmt.Printf("Found Drive : %v\n", *result.GetId())

This completely breaks the fluent API design, which is a tradeoff decision we could make to drastically improve performance.
I haven't implemented this generation "style" yet because it represents a change in direction that needs to be discussed further.

Also uploading my POC
selectiveimportsgo.zip

@baywet
Copy link
Member

baywet commented Dec 6, 2022

Update: since this initial investigation changes in the way the packages are structured yielded great build times improvements. closing for now.

@baywet baywet closed this as completed Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants