-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Updates for DOMKit #174
Updates for DOMKit #174
Conversation
Time Change: -35.5ms (8%) ✅ Total Time: 402.5ms
|
I like No longer supporting Not sure about Aliasing for
Curious to see some example code for this and to hear what @kateinoigakukun thinks about this change. |
An update on the JSObject changes (see swiftwasm/WebAPIKit#10): I no longer think the subclassing capability (and therefore the |
I have some objections to this.
I don't understand the necessity of this change, could you elaborate on the reason? protocol P0 {}
extension Array: P0 where Element == P0 {}
extension Dictionary: P0 where Value == P0, Key == String {}
// If forcing `Element` or `Value` to be non-existential, it breaks existential cases
// extension Array: P0 where Element: P0 {}
// extension Dictionary: P0 where Value: P0, Key == String {}
func takeP0<X: P0>(_: X) {}
func giveExistentialArray(_ xs: [P0]) {
takeP0(xs)
}
func giveGenericArray<X: P0>(_ xs: [X]) {
takeP0(xs)
}
func giveExistentialDictionary(_ xs: [String: P0]) {
takeP0(xs)
}
func giveGenericArray<X: P0>(_ xs: [String: X]) {
takeP0(xs)
}
Is this feature necessary only if we will take subclassing approach?
As far as I understand, |
Oh cool, I didn’t know about that. We should definitely use public protocol JSBridgedType: JSValueCompatible, CustomStringConvertible {
// renamed from `value`
var jsValue: JSValue { get }
init?(from jsValue: JSValue)
}
public protocol ConvertibleToJSValue {
// renamed from `jsValue` to avoid an autocomplete conflict
func toJSValue() -> JSValue
} Alternatively, we could have a unified API: public protocol JSBridgedType: JSValueCompatible, CustomStringConvertible {
// [the jsValue member is inherited from ConvertibleToJSValue]
init?(from jsValue: JSValue)
}
public protocol ConvertibleToJSValue {
var jsValue: JSValue { get }
} Note that:
Unfortunately, it isn’t. While you can call … sorry that was kinda badly written but hopefully it gets my point across! I only made this change because my build of DOMKit was breaking.
Yep, and I’ve pushed an update to remove it.
👍 |
It looks like fine to me 👍
Yes, |
The issue was that there’s a property wrapper for JS object keys: @propertyWrapper public struct ReadWriteAttribute<Wrapped: JSValueCompatible> {
// ...
}
class Foo: JSValueCompatible {
// ...
@ReadWriteAttribute var bar: [String]
} This did not compile, because |
Do you have a preference for it over the second proposal? I feel like the second one is more ergonomic. |
Ah, I got it. Makes sense to me to change the conformance condition.
I prefer the second one. |
Ok, I think I’m done making changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems legit to me, thanks!
This reverts commit 95d0c4c.
I'm actually finding this leading to quite hard to diagnose type checker issues, especially in Tokamak. Could this decision be reconsidered? WDYT an alternative could be? |
For reference, I'm trying to resolve build issues in TokamakUI/Tokamak#471 |
It's definitely up for reconsideration. One option would be I guess to make special property wrappers for arrays/dictionaries of JSValueCompatible things on the DOMKit side. Otherwise there isn’t an alternative that I can think of. |
[breaking] The
ConvertibleToJSValue
andJSBridgedType
protocols have changed:ConvertibleToJSValue
, you must now provide ajsValue: JSValue { get }
property instead of ajsValue() -> JSValue
method.ConvertibleToJSValue
will still work, as there is a protocol extension that provides a deprecatedjsValue()
method.JSBridgedType
has dropped itsvalue
property requirement in favor of inheriting thejsValue
property fromConvertibleToJSValue
. To update your conforming types, simply renamevalue
tojsValue
.[breaking] The
ConvertibleToJSValue
conformance onArray
andDictionary
has been swapped from the== ConvertibleToJSValue
case to the: ConvertibleToJSValue
case.[String]
is nowConvertibleToJSValue
, but[ConvertibleToJSValue]
no longer conformsjsValue()
method still works in both cases.map { $0.jsValue() }
(ormapValues
) to get an array/dictionary ofJSValue
which you can then use asConvertibleToJSValue
.jsValue
to the end of all of the values in the array/dictionary literalAllow people to do
object[JSString(...)]!(args...)
String
keys, even though the other property accessors worked withJSString
s too.Lower the Xcode concurrency requirements so they work with the new backporting features
Implement
JSUInt8ClampedArray
(wrappingUint8ClampedArray
)DOMKit
usage is inImageData
, so perhaps it would be better to alias it toJSTypedArray<UInt8>
?Allow users to subclass JSObject(removed)fromJSValue
won’t work)Add a new(removed)JSObject(cloning: JSObject)
initializer, enabling people to point two JSObject instances at the same object.super
in aJSObject
subclass, and there is unfortunately no way to “transmute” a JSObject into a subclass after getting it fromthis requires a new(removed)swjs_retain
call to keep reference counts correct[breaking] rename the(reverted)JSPromise/value
API toJSPromise/get()
to make it actually possible to useJSBridgedType.value
Result/get()
JSBridgedType
requirement tojsValue
, but this would make autocomplete for that somewhat annoying.ConvertibleToJSValue/jsValue()
totoJSValue()
to matchConstructibleFromJSValue
’sJSValue/fromJSValue()
method.