kiwicom / orbit-swiftui

Kiwi.com Orbit design system library
MIT License
132 stars 23 forks source link

Concurrency safe Color extensions #715

Closed Parabak closed 11 months ago

Parabak commented 1 year ago

Fixing warnings when StrictConcurrency is enabled

I've modified the script and run it to see the output.

Parabak commented 1 year ago

I have to fix Storybook first

Parabak commented 1 year ago

@PavelHolec We need to find solution for that. static var color shares mutable state in different contexts and have to be fixed. In Swift6 it produces warning:

Reference to static property 'orangeNormal' is not concurrency-safe because it involves shared mutable state

Simple static let will work but not for Storybook where we allow alternate colors 🤷

I've tried to make it thread-safe:

extension DispatchQueue {

    static let atomicColor = DispatchQueue(
        label: "CustomColorSync",
        qos: .userInteractive,
        attributes: .concurrent
    )

}

@propertyWrapper struct Actor<Value: Sendable>: Sendable {

    private var _value: Value

    var wrappedValue: Value {
        get { DispatchQueue.atomicColor.sync { return _value } }
        set { DispatchQueue.atomicColor.sync { _value = newValue } }
    }

    init(wrappedValue: Value) {
        self._value = wrappedValue
    }
}

but that works 2 times slower on reading and 3 times slower on writing than normal access 🤷 It's too much for such case.

PavelHolec commented 12 months ago

@PavelHolec We need to find solution for that. static var color shares mutable state in different contexts and have to be fixed. In Swift6 it produces warning:

Reference to static property 'orangeNormal' is not concurrency-safe because it involves shared mutable state

Simple static let will work but not for Storybook where we allow alternate colors 🤷

I've tried to make it thread-safe:

extension DispatchQueue {

    static let atomicColor = DispatchQueue(
        label: "CustomColorSync",
        qos: .userInteractive,
        attributes: .concurrent
    )

}

@propertyWrapper struct Actor<Value: Sendable>: Sendable {

    private var _value: Value

    var wrappedValue: Value {
        get { DispatchQueue.atomicColor.sync { return _value } }
        set { DispatchQueue.atomicColor.sync { _value = newValue } }
    }

    init(wrappedValue: Value) {
        self._value = wrappedValue
    }
}

but that works 2 times slower on reading and 3 times slower on writing than normal access 🤷 It's too much for such case.

In that case, let's drop the support to override colors in the storybook.

Parabak commented 12 months ago

will update the MR

Parabak commented 12 months ago

Hey guys, Storybook is updated as well. Please review