-
-
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
Gracefully handle unavailable JSBridgedClass
#190
Conversation
in the naming. Parts of the JavaScript `Date` API that are not consistent across browsers and JS | ||
implementations are not exposed in a type-safe manner, you should access the underlying `jsObject` | ||
property if you need those. | ||
*/ |
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.
Fix applied by the formatter.
@@ -94,8 +94,8 @@ private func getObjectValuesLength(_ object: JSObject) -> Int { | |||
return Int(values.length.number!) | |||
} | |||
|
|||
extension JSValue { | |||
public var array: JSArray? { | |||
public extension JSValue { |
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.
Fix applied by the formatter, as the rest of similar changes here.
Time Change: +140ms (1%) Total Time: 13,963ms
View Unchanged
|
Trying this in swiftwasm/WebAPIKit#41, looks like there's more work to do. Marking this PR as a draft, but I'm still looking forward to any feedback on this as a general approach. |
@@ -1,10 +1,10 @@ | |||
/// A wrapper around [the JavaScript Array class](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Array) | |||
/// that exposes its properties in a type-safe and Swifty way. | |||
public class JSArray: JSBridgedClass { | |||
public static let constructor = JSObject.global.Array.function! | |||
public static let constructor = JSObject.global.Array.function |
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.
For types like these where the global must be available, how do you feel about declaring constructor
as JSFunction!
?
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.
I tried this, but then you have to add an explicit JSFunction?
type signature on top, which means you still have to unwrap with !
or ?
wherever this constructor
is used. So after all, there's no benefit of unwrapping it and then wrapping back again into JSFunction?
to satisfy the protocol requirement.
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.
Ah, I had remembered the Foo!
and Foo?
types being compatible with each other but it seems that isn’t the case (i.e. you can’t conform to a protocol with a Foo?
requirement using a member of type Foo!
). So this seems like the best approach. 👍
Overall I like this! I had been contemplating adding a Boolean |
4639fb8
to
11c6898
Compare
I've verified that this fully works with WebAPIKit, ready for review. |
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.
Ah, I missed this PR
Updating the API to make it compatible with swiftwasm/JavaScriptKit#190. * Gracefully handle unavailable APIs * Update `Package.resolved` * Use JSKit 0.15.0
Force unwrapping in
class var constructor
may crash for classes that are unavailable in the current environment. For example, after swiftwasm/WebAPIKit#38 was merged, it led to crashes ingetCanvas
calls due to these optional casts relying onclass var constructor
always succeeding:if let gpuCanvasContext: GPUCanvasContext = value.fromJSValue()
branch crashes on browsers that don't haveGPUCanvasContext
enabled.As we currently don't have a better way to handle unavailable features, I propose making the result type of
static var constructor
requirement optional. This means you can still declare classes that are unavailable in the host JS environment. Conditional type casts are also available as they were, they will just always returnnil
, and initializers for these classes will returnnil
as well.