krzysztofzablocki / Inject

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

[InjectionIII.app] Bundle not found #65

Closed MaatheusGois closed 1 year ago

MaatheusGois commented 1 year ago
johnno1962 commented 1 year ago

👍

mbger commented 1 year ago

This change causes a crash when running DEBUG builds on physical devices because of the failing assertion. I guess I'm holding it wrong?

MaatheusGois commented 1 year ago

We can set a flag #if (Simulator) to resolve this issue. @mbger or change to print in console, I will do a implementation

mbger commented 1 year ago

Yeah I was experimenting in this direction yesterday. Basically I tried to achieve the following:

Introduce a new conditional compilation flag DISABLE_INJECT and modify the #if flags in Injection, i.e. something along those lines:

#if DEBUG && !DISABLE_INJECT
private var loadInjectionImplementation: Void = {
    guard objc_getClass("InjectionClient") == nil else { return }
#if os(macOS)
    let bundleName = "macOSInjection.bundle"
#elseif os(tvOS)
    let bundleName = "tvOSInjection.bundle"
#elseif targetEnvironment(simulator)
    let bundleName = "iOSInjection.bundle"
#elseif targetEnvironment(macCatalyst)
    let bundleName = "macOSInjection.bundle"
#else
    let bundleName = "maciOSInjection.bundle"
#endif // OS and environment conditions

    if let bundle = Bundle(path: "/Applications/InjectionIII.app/Contents/Resources/" + bundleName) {
        bundle.load()
    } else {
        assertionFailure("InjectionIII not found, verify if it's in /Applications")
    }
}()

Then my app target which uses Inject could define DISBABLE_INJECT whenever it's not building for the simulator and add it to SWIFT_ACTIVE_COMPILATION_CONDITIONS. But, alas, SwiftPM doesn't seem to support that.

johnno1962 commented 1 year ago

The conditional you are looking for is #if targetEnvironment(simulator) || os(macOS) but just make it a print with a ⚠️ instead of an assertionFailure. Let's keep it simple.

mbger commented 1 year ago

[...] but just make it a print with a ⚠️ instead of an assertionFailure. Let's keep it simple.

For sure. I had the impression that the whole point of the PR was to crash under those circumstances. Thanks for clarifying!

johnno1962 commented 1 year ago

Thanks for picking this up and reporting it @mbger. I guess the PR was intended to provide more feedback when the InjectionII app was not installed rather than silently failing to enable reloading as was the case. Using an assertionFailure however took it to the opposite extreme when you are running on a device. I think using a print statement would be a good compromise but it's your call @krzysztofzablocki.