Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

JS Exception Support#102

Merged
kateinoigakukun merged 5 commits into
mainswiftwasm/JavaScriptKit:mainfrom
katei/exceptionswiftwasm/JavaScriptKit:katei/exceptionCopy head branch name to clipboard
Oct 23, 2020
Merged

JS Exception Support#102
kateinoigakukun merged 5 commits into
mainswiftwasm/JavaScriptKit:mainfrom
katei/exceptionswiftwasm/JavaScriptKit:katei/exceptionCopy head branch name to clipboard

Conversation

@kateinoigakukun

@kateinoigakukun kateinoigakukun commented Oct 22, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

Currently, there is no way to catch exceptions thrown by JavaScript functions using JavaScriptKit.

This PR introduces new two APIs to catch exceptions.

  1. JSFunction.throws
  2. JSObject.throwing

JSFunction.throws converts JSFunction into throwable function. JSObject.throwing provides throwing method call API capturing this context.

These two APIs have different names to avoid overload ambiguity.

e.g.

// Throwing function call
let fn1: JSFunction = ...
try fn1.throws()

// Throwing constructor call
let Animal: JSFunction = ...
let cat1 = Animal.throws.new()

// Throwing method call
let obj1: JSObject = ...
obj1.throwing.method1!()

@github-actions

github-actions Bot commented Oct 22, 2020 •
edited
Loading

Copy link
Copy Markdown

Time Change: +75.5ms (0%)

Total Time: 10,698ms

Test name Duration Change
Serialization/Write JavaScript string directly 182.5ms -10.5ms (5%) ✅
ℹ️ View Unchanged
Test name Duration Change
Serialization/Write JavaScript number directly 183.5ms +8.5ms (4%)
Serialization/Swift Int to JavaScript 3,505ms -45ms (1%)
Serialization/Swift String to JavaScript 3,554.25ms -3.25ms (0%)
Object heap/Increment and decrement RC 3,272.75ms +125.75ms (3%)

performance-action

@kateinoigakukun kateinoigakukun marked this pull request as ready for review October 22, 2020 15:55
MaxDesiatov
MaxDesiatov approved these changes Oct 22, 2020
View reviewed changes

@MaxDesiatov MaxDesiatov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, awesome work!

@kateinoigakukun kateinoigakukun requested a review from j-f1 October 23, 2020 05:01
j-f1
j-f1 reviewed Oct 23, 2020
View reviewed changes
Comment thread Runtime/src/index.ts Outdated
Comment thread Runtime/src/index.ts
Comment thread Runtime/src/index.ts Outdated
let result: any
try {
result = Reflect.construct(obj, decodeValues(argv, argc));
if (typeof result != "object")

@j-f1 j-f1 Oct 23, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m pretty sure new can’t return a non-object. If you try to return a non-object in a constructor, it will use the this value instead.

See the spec here, where step 12 is the most relevant.

@kateinoigakukun

kateinoigakukun commented Oct 23, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

@j-f1 @MaxDesiatov What do you think about this API? I think there may be better interface.

@MaxDesiatov

MaxDesiatov commented Oct 23, 2020

Copy link
Copy Markdown
Member

If there's a better API for it, I can't come up with it...

j-f1
j-f1 approved these changes Oct 23, 2020
View reviewed changes

@j-f1 j-f1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! We’re not at 1.0 yet so we can continue to iterate if necessary.

@kateinoigakukun kateinoigakukun merged commit 6f31fc3 into main Oct 23, 2020
@kateinoigakukun kateinoigakukun deleted the katei/exception branch October 23, 2020 16:41
@MaxDesiatov MaxDesiatov mentioned this pull request Nov 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

@j-f1 j-f1 j-f1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kateinoigakukun @MaxDesiatov @j-f1
Morty Proxy This is a proxified and sanitized view of the page, visit original site.