Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@FlowInterop.Disabled does not work in the constructor #97

Open
X1nto opened this issue Jul 30, 2024 · 5 comments
Open

@FlowInterop.Disabled does not work in the constructor #97

X1nto opened this issue Jul 30, 2024 · 5 comments

Comments

@X1nto
Copy link

X1nto commented Jul 30, 2024

Questions:

What is the problem?

Applying @FlowInterop.Disabled to a constructor property still generates initializers with SkieSwiftFlow.

When does the problem occur?

During Swift compilation(?)
The initializer in the generated header does use Kotlinx_coroutines_coreFlow, but Xcode fails to see coreFlow and still treats it as SkieFlow

image image

How do we reproduce the issue?

Add this class to the iosMain source set of the shared module:

class Cockroach<T: Any>(
	@FlowInterop.Disabled
	private val flow: Flow<T>
)

Try using the class from Swift:

let cockroach = Cockroach(flow: ) //This flow initializer should accept Kotlinx_coroutines_coreFlow, but it's SkieSwiftFlow.
image

What has changed since the last time SKIE worked in your project?

If only I knew.

What versions of SKIE, Kotlin, and Gradle do you use?

Skie 0.8.3
Kotlin 2.0.0
Gradle 8.9
Xcode 15.4

What is your SKIE Gradle configuration?

Default configuration, no skie {} block.

There must be something that I'm missing. I've been going crazy these past 7 hours trying to fix this problem. The generated header being correct is what makes me actually mad, since it's probably some obscure Xcode bug that no one has ever encountered.

@TadeasKriz
Copy link
Collaborator

Hi @X1nto, thanks for the report. The header always shows Kotlinx_coroutines_coreFlow, because SKIE uses the .apinotes file to override the signature. If you look there you should see SkieKotlinFlow. As to why the annotation doesn't work, that's a good question. Could you try putting it on the constructor itself instead of the property?

class Cockroach<T: Any> @FlowInterop.Disabled constructor(
    private val flow: Flow<T>
)

Let me know if that changes anything.

@X1nto
Copy link
Author

X1nto commented Jul 30, 2024

My first thought was to put the annotation on the constructor, but the targets for the annotation are functions and properties.
image
image

@FilipDolnik
Copy link
Member

Hi! The annotation doesn't work because it targets the property, not the value parameter. Skie currently doesn't support Flow configuration for individual value parameters - it's a limitation of our current configuration framework.

As for why the annotation does not support constructors: I don't think it's intentional. We likely forgot to include the constructor target in there, so this is a "bug".

@X1nto
Copy link
Author

X1nto commented Jul 31, 2024

Do you accept PRs? I'll try and see if I can fix it

@FilipDolnik
Copy link
Member

Theoretically, yes, but we need to add tests as well, and those are not public, so a PR wouldn't save us any work in this case, unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants