Codegen Option: Optional Exported Imports about apollo-ios HOT 8 CLOSED

Thanks for the feedback @Mordil. Without the @exported import the generated models become pretty unwieldy. It means that you basically need to do add an import ApolloAPI in most of the places where you are using the models. So we don't think that creating yet another configuration option that allows you to disable this is really a good idea.

Though we do agree that ApolloAPI is exposing too much right now, which creates the type conflicts you're seeing and pollutes code completion. The solution we hope to implement for that that is using the @_spi attribute. This should allow us to only expose publicly the narrowly scoped parts of ApolloAPI that are helpful for consumers, while hiding the rest behind the SPI.

Because @_spi is still an underscored attribute, we've put off doing this, in hopes that its implementation would be finalized sooner than later. But I think we should probably move this up on our list of priorities and go ahead with the underscored attribute.

Additionally, we are currently working on this issues which will allow you to rename the generated names of schema types, including custom scalars. This will help you get around the naming conflicts with things like Foundation.Date.

from apollo-ios.

That doesn't necessarily help us with conflicts between ApolloAPI.Object and Realm.Object, however.

I understand the rationale for the default as it is, because it does simplify using things as you described by not requiring import ApolloAPI - however, this is no less burdensome than those of us in this situation who now have to full-specify all of our types from these conflicts anyway, and arguably makes it worse.

from apollo-ios.

Yeah, that definitely doesn't feel great either right now. FWIW, there is some pretty extensive discussion of this topic in this issue already. But I don't think ApolloAPI.Object specifically existed at that time.

I think the parts of ApolloAPI that actually are better off exported are pretty small. GraphQLNullable and GraphQLEnum definitely should be, but I don't think ApolloAPI.Object should be exported anyways. So I think the way to handle this is to have an SPI that only contains the few things that really should be exported as well. We should ensure that anything in that SPI has names that are not too generalized and are unlikely to have lots of conflicts. GraphQLNullable can be exported pretty safely for 99% of people, but not Object.

from apollo-ios.

Basically we would have:

• @_spi(exports)
  • includes things that should be exported
• Just regular public,
  -entities like Object, that may be consumed by users in parts of their infrastructure that directly import ApolloAPI, but aren't necessary just for consuming the generated models.
• @_spi(internal)
  • Things that need to be used by the generated models internally, but should not be used by consumers of Apollo directly.

Then the generated models end up having their import statements look like:

@_exported @spi(exports) import ApolloAPI
@spi(internal) import ApolloAPI

Though I'm concerned that the compiler won't let that work, due to seeing it as duplicate import statements for ApolloAPI.
In which case, maybe the exported things need to be in their own new module ApolloAPIExports or something like that.

from apollo-ios.

@AnthonyMDev Wouldn't we want to have everything that would be useful to consumers still just be regular ApolloAPI with the @_exported import ApolloAPI as is, and then take everything that doesn't need to be exported by default or is internal and move those into @_spi's so that users can import them at will?

from apollo-ios.

There are a lot of parts of ApolloAPI that are useful for that parts of your app code that configure your networking and cache layer. Custom interceptors, cache key configuration, and custom cache mutations for example. Those parts should be public because there is use case for them and they are intended for public consumption. However they don't need to be exported to all of your other code that just consumes the generated models.

By exporting all of ApolloAPI, these components are exported everywhere and you get issues like the conflict between ApolloAPI.Object and Realm.Object. If those issues were isolated to only files that needed to work with them and explicitly import ApolloAPI, the namespacing becomes a lot less cumbersome. And for all the rest of your codebase, you don't get them coming up in code completion when they aren't appropriate.

There are also pieces that are not intended for public consumption, but are consumed by the generated models. These pieces should be part of the @_spi(internal).

The only pieces that need to be exported are the pieces that you need to use to interact with the generated models, like GraphQLEnum and GraphQLNullable. Which is why I'm recommending 2 different SPIs, ultimately giving us 3 different access levels that can be used for each of these use cases.

from apollo-ios.

Yea I was just recommending that maybe for things like GraphQLEnum and GraphQLNullable they remain as is with no spi so they can still be used through the existing @_exported import ApolloAPI` and then everything else can be broken down into however many spi's we feel are necessary to provide targeted imports users can call if needed.

from apollo-ios.

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better.

from apollo-ios.