krzysztofzablocki / Inject

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

TCA 1.7.3 crash (ComposableArchitecture/Store.swift:682: Fatal error: Unexpectedly found nil while unwrapping an Optional value) #87

Closed kuglee closed 5 months ago

kuglee commented 6 months ago

I'm trying to use this package with TCA 1.7.3, however when a non-leaf view is changed the app crashes. I've created an example project for this: InjectTCATest. In the linked project if I change the Text("AppFeature") in AppView to Text("AppFeature2") there's a crash:

πŸ’‰ InjectionIII connected /Users/kuglee/Downloads/InjectTCATest/InjectTCATest.xcodeproj
πŸ’‰ Watching files under the directory /Users/kuglee/Downloads/InjectTCATest
πŸ’‰ Compiling /Users/kuglee/Downloads/InjectTCATest/InjectTCATest/AppFeature.swift
πŸ’‰ Selecting Xcode /Applications/Xcode-beta.app/Contents/Developer
πŸ’‰ Loading .dylib ...
πŸ’‰ Interposed 15 function references.
πŸ’‰ Injected type #1 'InjectTCATest.AppFeature'
πŸ’‰ Injected type #2 'InjectTCATest.AppFeature.State'
πŸ’‰ Injected type #3 'InjectTCATest.AppFeature.Action'
πŸ’‰ Injected type #4 'InjectTCATest.AppFeature.Action.AllCasePaths'
πŸ’‰ Injected type #5 'InjectTCATest.AppView'
ComposableArchitecture/Store.swift:682: Fatal error: Unexpectedly found nil while unwrapping an Optional value
crash

If I change the Text("ChildFeature") in ChildView to Text("ChildFeature2") it works as expected.

johnno1962 commented 6 months ago

Hi, a top level variable could be getting reinitialised on injection. Can you try setting an environment variable in your scheme: INJECTION_PRESERVE_STATICS ? Any value will do.

kuglee commented 6 months ago

I've set the environment variable. Unfortunately the same crash still occurs.  INJECTION_PRESERVE_STATICS

johnno1962 commented 6 months ago

I'm afraid I can't tell you why but if you move the AppFeature struct into the ChildFeature.swift file things work. It's better to have types in their own files with injection.

krzysztofzablocki commented 6 months ago

Yeah that's my experience too, try to keep type per file and not multiple related types in single file, maybe something to do with order of pathing @johnno1962 ?

johnno1962 commented 6 months ago

Patching? If you move the types around in the file it doesn't seem to help. These are suddenly complicated files with all the macros that are creating static member variables so it's not clear what's going on. It's a shame but the first thing to try when you get this type of crash is separating out/moving around the types.

johnno1962 commented 6 months ago

Thanks @kuglee for your example project. It makes very clear what the problem was which is that the "key path getter functions" get injected and therefore the key path isn't recognised as being the same inside the TCA code resulting in the force unwrap 😱 failing. For now, I can't get "reverse interposing" which would be a solution for this working so it's not possible to inject a type, the fields of which are referenced using key paths i.e you can't inject a file containing the AppFeature struct.

kuglee commented 6 months ago

That's a bummer. Anyway, thank you for looking into this.

johnno1962 commented 5 months ago

@kuglee, I may have found a solution of sorts for this problem if you want to try the new release candidate. The fix is enabled if you set an INJECTION_KEYPATHS environment variable in your scheme. Thanks for creating the test project - It was a great help working out where the problem was.

kuglee commented 5 months ago

If I set the INJECTION_KEYPATHS, I have the following crash:

xcode

shceme

johnno1962 commented 5 months ago

Thanks for trying it out so quickly. Which version of Xcode is this and which build number of the InjectionIII.app? (It's in the Info.plist - it should be 8012)

kuglee commented 5 months ago

Xcode: Version 15.3 (15E204a) InjectionIII: 4.8.4

kuglee commented 5 months ago

InjectionIII: 8012

johnno1962 commented 5 months ago

I've uploaded a new 4.8.4 release candidate, this time build 8014. Can you give it a try? There was an iOS specific problem.

kuglee commented 5 months ago

I've tried 8014. The original issue still occurs:

xcode

shceme

johnno1962 commented 5 months ago

Disappointing. And the env var was set? What are you trying to do? Start with just saving the AppFeature.swift file and work you way up from there to see what is possible now.

kuglee commented 5 months ago

I've pushed an update to the sample project with the environment variables.

I've changed the Text("AppFeature") in AppView to Text("AppFeature2") just like in the first example when the crash occurred.

johnno1962 commented 5 months ago

OK, I've updated the RC to build 8015. The iOS specific problem was more involved than I thought. Can you give it a try please? You should see a message πŸ’‰ Recycling ... if it is working.

kuglee commented 5 months ago

It works now for iOS and also macOS! Thanks for taking the time πŸ™

johnno1962 commented 5 months ago

Great! Yes, this one was a real time consumer but having the Feature and View in the same file is the norm for TCA so I really wanted to be sure it worked even if people have to remember to opt in with the env var for now. There will be limits and if you run up against something let me know. If you don't, remember there's a tip jar on the InjectionIII project!