-
-
Notifications
You must be signed in to change notification settings - Fork 57
Updates for DOMKit #174
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
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. |
MaxDesiatov
left a comment
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
ConvertibleToJSValueandJSBridgedTypeprotocols have changed:ConvertibleToJSValue, you must now provide ajsValue: JSValue { get }property instead of ajsValue() -> JSValuemethod.ConvertibleToJSValuewill still work, as there is a protocol extension that provides a deprecatedjsValue()method.JSBridgedTypehas dropped itsvalueproperty requirement in favor of inheriting thejsValueproperty fromConvertibleToJSValue. To update your conforming types, simply renamevaluetojsValue.[breaking] The
ConvertibleToJSValueconformance onArrayandDictionaryhas been swapped from the== ConvertibleToJSValuecase to the: ConvertibleToJSValuecase.[String]is nowConvertibleToJSValue, but[ConvertibleToJSValue]no longer conformsjsValue()method still works in both cases.map { $0.jsValue() }(ormapValues) to get an array/dictionary ofJSValuewhich you can then use asConvertibleToJSValue.jsValueto the end of all of the values in the array/dictionary literalAllow people to do
object[JSString(...)]!(args...)Stringkeys, even though the other property accessors worked withJSStrings too.Lower the Xcode concurrency requirements so they work with the new backporting features
Implement
JSUInt8ClampedArray(wrappingUint8ClampedArray)DOMKitusage is inImageData, so perhaps it would be better to alias it toJSTypedArray<UInt8>?Allow users to subclass JSObject(removed)fromJSValuewon’t work)Add a new(removed)JSObject(cloning: JSObject)initializer, enabling people to point two JSObject instances at the same object.superin aJSObjectsubclass, and there is unfortunately no way to “transmute” a JSObject into a subclass after getting it fromthis requires a new(removed)swjs_retaincall to keep reference counts correct[breaking] rename the(reverted)JSPromise/valueAPI toJSPromise/get()to make it actually possible to useJSBridgedType.valueResult/get()JSBridgedTyperequirement tojsValue, but this would make autocomplete for that somewhat annoying.ConvertibleToJSValue/jsValue()totoJSValue()to matchConstructibleFromJSValue’sJSValue/fromJSValue()method.