skip icon indicating copy to clipboard operation
skip copied to clipboard

Code in `#if !os(macOS)` conditional compilation blocks doesn't show up on Android

Open dfabulich opened this issue 10 months ago • 4 comments

In https://github.com/skiptools/skipapp-showcase/pull/21 I fixed the Showcase app to build on native macOS (without Catalyst).

In the course of doing that, I wrapped some modifiers in #if !os(macOS) blocks, like this:

https://github.com/skiptools/skipapp-showcase/pull/21/files#diff-1d95ee84bfd664e75e03a2be16217e70cd10cf783117b7853c61c88170eea347R57-R59

#if !os(macOS)
.keyboardType(UIKeyboardType.phonePad)
#endif

Expected: When I do that, the soft keyboard should use the phone keyboard on this field for both iOS and Android

Actual: The soft keyboard on iOS uses the phone keyboard, but on Android, it uses the default keyboard.

dfabulich avatar Feb 17 '25 20:02 dfabulich

I inspected the generated Kotlin of TextFieldPlayground.kt.

On the main branch of skipapp-showcase at 721c45253fe34092b61b78de3a14d10cf336bd66 TextFieldPlayground.kt looks like this:

TextField(LocalizedStringKey(stringLiteral = "(###) ###-####"), text = Binding({ _phone.wrappedValue }, { it -> _phone.wrappedValue = it }))
    .textFieldStyle(TextFieldStyle.plain)
    .keyboardType(UIKeyboardType.phonePad)
    .onChange(of = phone) { newValue -> phone = formatPhone(newValue) }.Compose(composectx)

In my PR branch, TextFieldPlayground.kt looks like this:

TextField(LocalizedStringKey(stringLiteral = "(###) ###-####"), text = Binding({ _phone.wrappedValue }, { it -> _phone.wrappedValue = it }))
    .textFieldStyle(TextFieldStyle.plain)
    .onChange(of = phone) { newValue -> phone = formatPhone(newValue) }.Compose(composectx)

As I understand it, adding a #if !os(macOS) conditional compiler block should not affect this generated Kotlin, so this is a transpiler bug.

dfabulich avatar Feb 17 '25 20:02 dfabulich

https://github.com/skiptools/skipapp-showcase/pull/21#issuecomment-2664021261

@marcprux wrote:

The problem is that Skip's preprocessor logic interpretation isn't all that smart: we don't handle much other than #if SKIP and #if os(Android) … everything else just gets evaluated to false (the ones listed at https://skip.tools/docs/platformcustomization/#compiler-directives are all we try to handle).

So what you can instead do is just insert an empty #if os(macOS) block, and then put the iOS+Android code in the #else block, like we do in PickerPlayground.swift:

https://github.com/skiptools/skipapp-showcase/blob/721c45253fe34092b61b78de3a14d10cf336bd66/Sources/Showcase/PickerPlayground.swift#L140-L143

That workaround won't work in this case.

When I write this Swift:

TextField("(###) ###-####", text: $phone)
    .textFieldStyle(.plain)
    #if os(macOS)
    #else
    .keyboardType(UIKeyboardType.phonePad)
    #endif
    .onChange(of: phone) { newValue in
        phone = formatPhone(newValue)
    }

It fails to compile: "Type 'Any' has no member 'keyboardType'"

dfabulich avatar Feb 17 '25 20:02 dfabulich

That workaround won't work in this case.

Oh yeah. I guess you could do something like this:

extension View {
    func keyboardTypePhonePad() -> some View {
        #if os(macOS)
        return self
        #else
        return self.keyboardType(UIKeyboardType.phonePad)
        #endif
    }
}

TextField("(###) ###-####", text: $phone)
    .textFieldStyle(.plain)
    .keyboardTypePhonePad()
    .onChange(of: phone) { newValue in
        phone = formatPhone(newValue)
    }

Or you could just #if out the whole view for macOS and just have it not show up in the macOS version of the playground…

marcprux avatar Feb 17 '25 20:02 marcprux

I found a better workaround, #if !os(macOS) || os(Android), and filed a documentation bug #343 on this.

Still, I really think this bug should be fixed, so users don't have to understand/workaround this issue. If the transpiler can understand os(Android), then it should be able to understand that os(whatever) means !os(Android), and !os(whatever) means os(Android).

The Swift book says that there are only seven valid arguments to os() and only five kinds of platform conditions. It's not a huge stretch for Skip to at least understand all of these.

Platform condition Valid arguments
os() macOS, iOS, watchOS, tvOS, visionOS, Linux, Windows
arch() i386, x86_64, arm, arm64
swift() >= or < followed by a version number
compiler() >= or < followed by a version number
canImport() A module name
targetEnvironment() simulator, macCatalyst

dfabulich avatar Feb 17 '25 21:02 dfabulich