krzysztofzablocki / Inject

Hot Reloading for Swift applications!
MIT License
2.1k stars 114 forks source link

'Inject.Inject' shadows module 'Inject' - Inject does not compile when used in framework #82

Open cakl opened 7 months ago

cakl commented 7 months ago

Starting with Xcode_14.3 it seems that it is not possible to use Inject in a framework as the following build error appears:

.../SourcePackages/checkouts/Inject/Sources/Inject/Inject.swift:15:13: warning: public enum 'Inject.Inject' shadows module 'Inject', which may cause failures when importing 'Inject' or its clients in some configurations; please rename either the enum 'Inject.Inject' or the module 'Inject', or see https://github.com/apple/swift/issues/56573 for workarounds

When built with Xcode_14.2 there is just a warning about the shadowing.

Sample project: https://github.com/cakl/MyUIFramework (succeeds with Xcode_14.2, fails with Xcode_14.3.1 or higher).

Similar issue: https://github.com/krzysztofzablocki/Difference/issues/36

I know that there are workarounds (https://github.com/apple/swift/issues/64669#issuecomment-1535335601) but they come with a "high price" as far as I understand them.

Thanks for any help

krzysztofzablocki commented 7 months ago

We can rename Inject to InjectConfiguration and remove namespacing so that Hosts are not namespaced so they can still be referred to by Inject.VIewControllerHost etc

johnno1962 commented 7 months ago

Idle thought, do you get this warning if you use a typealias?

cakl commented 7 months ago

Idle thought, do you get this warning if you use a typealias?

A typealias on the integrating side like this: https://github.com/cakl/MyUIFramework/blob/feature/typealias/MyUIFramework/ViewController.swift ? This does not seem to help.

johnno1962 commented 7 months ago

Are we sure this is still an issue? I have a class SwiftTrace in a package SwiftTrace which used to give a warning but I've not seen it for a while (Xcode 15). There was talk for a while this would eventually not be allowed but perhaps they realised this would be very disruptive.

cakl commented 7 months ago

Are we sure this is still an issue? I have a class SwiftTrace in a package SwiftTrace which used to give a warning but I've not seen it for a while (Xcode 15). There was talk for a while this would eventually not be allowed but perhaps they realised this would be very disruptive.

I only have the issue if Inject is used in a framework. If I'm not doing anything else wrong, yes, it is still an issue. The behaviour can be reproduced with the linked sample project (https://github.com/cakl/MyUIFramework) with Xcode 14.2 and Xcode 14.3.1 (build the xcframework with the provided shell file). The same behaviour is also reproduceable on GH runners (14.2 build vs 14.3.1 build)

johnno1962 commented 7 months ago

OK, I'm with you now. Using your script, I can see that when packaging a binary framework an invalid .swiftinterface file is generated if a type shares its name with the package which is what Xcode warns about - even with Xcode 15.2.

johnno1962 commented 7 months ago

Can you not add OTHER_SWIFT_FLAGS=-no-verify-emitted-module-interface to your script? It builds but I don't know if that just kicks the problem down the road. The invalid Inject.swiftinterface doesn't seem to be in the built .xcframework.

johnno1962 commented 7 months ago

We've not heard back from you. Are you not able to build your framework? I'm using the following and it seems to work:


#!/bin/bash
set -euxo pipefail

# Builds xcframework from iOS framework template project called MyUIFramework

# Clean build folder
rm -rf ./build

# Archive for iOS
xcodebuild archive -scheme MyUIFramework -destination="iOS" -archivePath ./build/ios.xcarchive -sdk iphoneos SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES OTHER_SWIFT_FLAGS=-no-verify-emitted-module-interface

# Archive for simulator
xcodebuild archive -scheme MyUIFramework -destination="iOS Simulator" -archivePath ./build/iossimulator.xcarchive -sdk iphonesimulator SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES OTHER_SWIFT_FLAGS=-no-verify-emitted-module-interface

# Build xcframework with two archives
xcodebuild -create-xcframework -framework ./build/ios.xcarchive/Products/Library/Frameworks/MyUIFramework.framework -framework ./build/iossimulator.xcarchive/Products/Library/Frameworks/MyUIFramework.framework -output ./build/MyUIFramework.xcframework
cakl commented 7 months ago

We've not heard back from you. Are you not able to build your framework? I'm using the following and it seems to work:

#!/bin/bash
set -euxo pipefail

# Builds xcframework from iOS framework template project called MyUIFramework

# Clean build folder
rm -rf ./build

# Archive for iOS
xcodebuild archive -scheme MyUIFramework -destination="iOS" -archivePath ./build/ios.xcarchive -sdk iphoneos SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES OTHER_SWIFT_FLAGS=-no-verify-emitted-module-interface

# Archive for simulator
xcodebuild archive -scheme MyUIFramework -destination="iOS Simulator" -archivePath ./build/iossimulator.xcarchive -sdk iphonesimulator SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES OTHER_SWIFT_FLAGS=-no-verify-emitted-module-interface

# Build xcframework with two archives
xcodebuild -create-xcframework -framework ./build/ios.xcarchive/Products/Library/Frameworks/MyUIFramework.framework -framework ./build/iossimulator.xcarchive/Products/Library/Frameworks/MyUIFramework.framework -output ./build/MyUIFramework.xcframework

Sorry for the delayed reply. I will try your script in the production code asap. I just thought that this build flag should be the last option and hoped that @krzysztofzablocki’s proposal of renaming should be the way to go (to prevent to „kick the problem down the road“).

johnno1962 commented 7 months ago

I'd go for trying changing your script for your specific use case before we start renaming things which might affect other Inject users.

cakl commented 4 months ago

@krzysztofzablocki Sorry for the late reply and thank you for your changes.

What I observed:

I may not fully understand Inject's code and your change, but to me it looks like in the release build the hosts must be accessed via InjectConfiguration (e.g. InjectConfiguration.ViewControllerHost(...)) and in a Debug build still Inject.ViewControllerHost(...) can be used (because of the global typealias). Or what am I missing?

Thanks for your help...

Frizlab commented 1 month ago

I noticed the same as @cakl with latest version (1.5.2).

krzysztofzablocki commented 1 month ago

@cakl you are right, we need to match up the interface to be 1:1 with debug vs release, I missed this when doing those changes, I won't get to it until next week since I've family coming over the weekend but if either of you would want to contribute PR with it I'd be able to review and merge before hand

Frizlab commented 1 month ago

I created a PR. I’m not sure this is the right fix, but it looks ok.