google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.8k stars 294 forks source link

Usage of private FBLPromise+Testing.h inside swift Promises #123

Closed pilot34 closed 5 years ago

pilot34 commented 5 years ago

Hi guys,

Seems like we have some inconsistency:

I'm not sure how does that work with CocoaPods. Seems like the private header is exposed as a public header there. Because if we use FBLPromises.h as an umbrella header, it doesn't include FBLPromise+Testing.h and swift code doesn't compile.

What do you think is the best solution? Move Testing.h to the public? Or maybe add private header inside the Swift framework?

shoumikhin commented 5 years ago

Hi Gleb,

+Testing header is listed in and exposed via the custom module-map file. But it’s intentionally omitted in the umbrella header. That way the FBLPromises framework has it provided and the Swift counterpart can use it, but the normal users won’t see it when importing the umbrella, unless they import the +Testing header directly.

Do you face any specific building issues with CocoaPods? That’s likely because the module-map is generated to expose all headers listed in the umbrella, so it doesn’t contain the +Testing one. But that’s just a guess.

Thanks.

pilot34 commented 5 years ago

Similar problem. I'm building with Buck, it takes FBLPromises.h as a header for module-map.

So in your current setup with CocoaPods, if we do @import FBLPromises; in objective-c, the private header will also be visible, right? And it is visible in swift not only for Promises framework but for everybody. Looks like not a good way to hide a private header, IMHO. Modular @imports are common in obj-c now.

But the only solution can be to do a single framework for both, right?

The problem is that in our codebase some guys already used properties from private headers, that I think, is not safe. But I cannot forbid to do that in the current setup.

shoumikhin commented 5 years ago

Hi Gleb,

Currently we hide the FBLPromise+Testing.h by not exposing it in the umbrella header, but making it available to the Swift lib by listing it in the custom module map. Please let us know if you know a better way of "hiding" that header.

If the build system you're using doesn't support custom module maps (e.g. Buck), there's probably not much we can do. Or if we don't use some feature yet (e.g. CocoaPods), we are more than happy to consider a pull-request with a fix.

Here's a possible fix for PromisesObjC.podspec. Please test it locally and let us know if that's something you need.

And here's a rough BUCK file, in case you're gonna use Buck. There may be some issues with it, since I didn't test it properly. And as I mentioned above, Buck doesn't support custom module maps, so the +Testing header is still gonna stay exposed.

Anyhow, please keep us posted if you have further questions, and feel free to send PRs our way if there's a need to fix anything.

Thanks!

pilot34 commented 5 years ago

Thank you very much for the descriptive answer!

I don't think there can be a better way to hide the header. Only merging both frameworks to one can maybe help.