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

Sendable conformance #40

Closed
mcomisso opened this issue Jan 12, 2024 · 7 comments · Fixed by #86
Closed

Sendable conformance #40

mcomisso opened this issue Jan 12, 2024 · 7 comments · Fixed by #86
Labels
enhancement New feature or request

Comments

@mcomisso
Copy link

Thanks for open sourcing this!

Using it on some parts marked @MainActor. Xcode produces the following warnings:

Passing argument of non-sendable type 'LocalizedStringResource' outside of main actor-isolated context may introduce data races

Being static strings I think we should be able to make it @sendable? 🤔

@liamnichols
Copy link
Owner

Interesting! I haven’t used Swift Concurrency much myself yet so I’m not so familiar in this area, but I think that you are right.

Is this a case of me having to add Sendable conformance to the nested LocalizedStringResource.Localizable struct? The error seems to suggest that it’s an issue with LocalizedStringResource, but that’s a Foundation type and I don’t think I can add sendable directly to that?

@liamnichols liamnichols added the enhancement New feature or request label Jan 13, 2024
@liamnichols
Copy link
Owner

An Apple Frameworks Engineer specifies that LocalizedStringResource cannot be made Sendable 😕

https://forums.developer.apple.com/forums/thread/732362

Unfortunately I don't think that we can do anything about this from our side. I guess that you might need to load the resource into a String before passing it across isolation domains?

I'm going to close this issue for now, but please reopen it if you feel that there is something that can be done here. Thanks!

@liamnichols liamnichols closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
@mcomisso
Copy link
Author

Thank you for looking into this! And apologies for not replying the last few days.

That's a very interesting find!
I don't have enough experience on the subject, but I guess if not @Sendable the improvement could be to make it @MainActor to require the await when interacted in an unsafe context? 🤔

@liamnichols liamnichols reopened this Jan 24, 2024
@liamnichols
Copy link
Owner

I don't have enough experience on the subject, but I guess if not @sendable the improvement could be to make it @mainactor to require the await when interacted in an unsafe context? 🤔

Me neither 😄 Maybe I should leave this open a bit longer then..

Would it be possible for you to share a code example that surfaces the issue that you are describing? I'm not quite sure how to reproduce the warning that you are seeing.

@mcomisso
Copy link
Author

Sure, writing it by memory, but if I'm not wrong was something like this:

struct SomeView: View {
  var body: some View {
    myVarView
  }

  @MainActor
  @ViewBuilder
  var myVarView: some View {
    Text(.localizable.footerText(items.count))  // <- warning
  }
}

@liamnichols
Copy link
Owner

Sorry for the delay, but I tried replicating this in the demo app and I can't seem to reproduce the warning that you described.

If you get some time, please can you share a project that replicates the issue? Using the demo project might help if you can't share your own source code

@liamnichols
Copy link
Owner

When I opened the demo project in Xcode 15.3, I see this warning in the generated code:

Screenshot 2024-04-03 at 07 57 34

Seems to be related.. Will look into it.

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

Successfully merging a pull request may close this issue.

2 participants