-
-
Notifications
You must be signed in to change notification settings - Fork 221
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
Function Templates with callback functions in Go #68
Merged
Merged
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
2271276
refactor to extend from private template struct
rogchap acf4622
refactor the C++ side to also match base class template
rogchap cd0605a
Basic callbacks with arguments
rogchap 31522af
fix stat now there is an internal context
rogchap ff95610
deal with Go -> C pointer madness
rogchap 2b9591f
apply formatting and add examples
rogchap 31a02f4
add tests to the function template and the registries
rogchap 4796e0e
simplify, bug fixes and add comments
rogchap File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
package v8go | ||
|
||
// RegisterCallback is exported for testing only. | ||
func (i *Isolate) RegisterCallback(cb FunctionCallback) int { | ||
return i.registerCallback(cb) | ||
} | ||
|
||
// GetCallback is exported for testing only. | ||
func (i *Isolate) GetCallback(ref int) FunctionCallback { | ||
return i.getCallback(ref) | ||
} | ||
|
||
// Register is exported for testing only. | ||
func (c *Context) Register() { | ||
c.register() | ||
} | ||
|
||
// Deregister is exported for testing only. | ||
func (c *Context) Deregister() { | ||
c.deregister() | ||
} | ||
|
||
// GetContext is exported for testing only. | ||
var GetContext = getContext | ||
|
||
// Ref is exported for testing only. | ||
func (c *Context) Ref() int { | ||
return c.ref | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
package v8go | ||
|
||
// #include <stdlib.h> | ||
// #include "v8go.h" | ||
import "C" | ||
import ( | ||
"errors" | ||
"runtime" | ||
"unsafe" | ||
) | ||
|
||
// FunctionCallback is a callback that is executed in Go when a function is executed in JS. | ||
type FunctionCallback func(info *FunctionCallbackInfo) *Value | ||
|
||
// FunctionCallbackInfo is the argument that is passed to a FunctionCallback. | ||
type FunctionCallbackInfo struct { | ||
ctx *Context | ||
args []*Value | ||
} | ||
|
||
// Context is the current context that the callback is being executed in. | ||
func (i *FunctionCallbackInfo) Context() *Context { | ||
return i.ctx | ||
} | ||
|
||
// Args returns a slice of the value arguments that are passed to the JS function. | ||
func (i *FunctionCallbackInfo) Args() []*Value { | ||
return i.args | ||
} | ||
|
||
// FunctionTemplate is used to create functions at runtime. | ||
// There can only be one function created from a FunctionTemplate in a context. | ||
// The lifetime of the created function is equal to the lifetime of the context. | ||
type FunctionTemplate struct { | ||
*template | ||
} | ||
|
||
// NewFunctionTemplate creates a FunctionTemplate for a given callback. | ||
func NewFunctionTemplate(iso *Isolate, callback FunctionCallback) (*FunctionTemplate, error) { | ||
if iso == nil { | ||
return nil, errors.New("v8go: failed to create new FunctionTemplate: Isolate cannot be <nil>") | ||
} | ||
if callback == nil { | ||
return nil, errors.New("v8go: failed to create new FunctionTemplate: FunctionCallback cannot be <nil>") | ||
} | ||
|
||
cbref := iso.registerCallback(callback) | ||
|
||
tmpl := &template{ | ||
ptr: C.NewFunctionTemplate(iso.ptr, C.int(cbref)), | ||
iso: iso, | ||
} | ||
runtime.SetFinalizer(tmpl, (*template).finalizer) | ||
return &FunctionTemplate{tmpl}, nil | ||
} | ||
|
||
//export goFunctionCallback | ||
func goFunctionCallback(ctxref int, cbref int, args *C.ValuePtr, args_count int) C.ValuePtr { | ||
ctx := getContext(ctxref) | ||
|
||
info := &FunctionCallbackInfo{ | ||
ctx: ctx, | ||
args: make([]*Value, args_count), | ||
} | ||
|
||
argv := (*[1 << 30]C.ValuePtr)(unsafe.Pointer(args))[:args_count:args_count] | ||
for i, v := range argv { | ||
val := &Value{ptr: v} | ||
runtime.SetFinalizer(val, (*Value).finalizer) | ||
info.args[i] = val | ||
} | ||
|
||
callbackFunc := ctx.iso.getCallback(cbref) | ||
if val := callbackFunc(info); val != nil { | ||
return val.ptr | ||
} | ||
return nil | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
package v8go_test | ||
|
||
import ( | ||
"fmt" | ||
"io/ioutil" | ||
"net/http" | ||
"strings" | ||
"testing" | ||
|
||
"rogchap.com/v8go" | ||
) | ||
|
||
func TestFunctionTemplate(t *testing.T) { | ||
t.Parallel() | ||
|
||
if _, err := v8go.NewFunctionTemplate(nil, func(*v8go.FunctionCallbackInfo) *v8go.Value { return nil }); err == nil { | ||
t.Error("expected error but got <nil>") | ||
} | ||
|
||
iso, _ := v8go.NewIsolate() | ||
if _, err := v8go.NewFunctionTemplate(iso, nil); err == nil { | ||
t.Error("expected error but got <nil>") | ||
} | ||
|
||
fn, err := v8go.NewFunctionTemplate(iso, func(*v8go.FunctionCallbackInfo) *v8go.Value { return nil }) | ||
if err != nil { | ||
t.Errorf("unexpected error: %v", err) | ||
} | ||
if fn == nil { | ||
t.Error("expected FunctionTemplate, but got <nil>") | ||
} | ||
} | ||
|
||
func ExampleFunctionTemplate() { | ||
iso, _ := v8go.NewIsolate() | ||
global, _ := v8go.NewObjectTemplate(iso) | ||
printfn, _ := v8go.NewFunctionTemplate(iso, func(info *v8go.FunctionCallbackInfo) *v8go.Value { | ||
fmt.Printf("%+v\n", info.Args()) | ||
return nil | ||
}) | ||
global.Set("print", printfn, v8go.ReadOnly) | ||
ctx, _ := v8go.NewContext(iso, global) | ||
ctx.RunScript("print('foo', 'bar', 0, 1)", "") | ||
// Output: | ||
// [foo bar 0 1] | ||
} | ||
|
||
func ExampleFunctionTemplate_fetch() { | ||
iso, _ := v8go.NewIsolate() | ||
global, _ := v8go.NewObjectTemplate(iso) | ||
fetchfn, _ := v8go.NewFunctionTemplate(iso, func(info *v8go.FunctionCallbackInfo) *v8go.Value { | ||
args := info.Args() | ||
url := args[0].String() | ||
res, _ := http.Get(url) | ||
body, _ := ioutil.ReadAll(res.Body) | ||
val, _ := v8go.NewValue(iso, string(body)) | ||
return val | ||
}) | ||
global.Set("fetch", fetchfn, v8go.ReadOnly) | ||
ctx, _ := v8go.NewContext(iso, global) | ||
val, _ := ctx.RunScript("fetch('https://rogchap.com/v8go')", "") | ||
fmt.Printf("%s\n", strings.Split(val.String(), "\n")[0]) | ||
// Output: | ||
// <!DOCTYPE html> | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
how does
vm.TerminateExecution
interact with callback ofv8go.NewFunctionTemplate
?would it be possible to include
context.Context
as first argument of callback?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.
Once the VM is terminated, you will no longer be able to fire the callback. These callbacks are connected to the isolate, so once the Isolate is GC'd so will be the callback functions (unless attached to some other struct etc)
You can get the current context via
info.Context()
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.
context.Context
will get confusing withv8go.Context
(as I just did)The V8 API does not have such a concept as the Go Context; if we introduce the Go Context, this would have to exist throughout the API, which would likely be a breaking change.
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 had a look at your code (https://github.com/choonkeat/try-v8go/blob/main/main.go)
I see what you are trying to do... you want to be able to check if the Go Context is
Done
within the callback?Maybe we could attach a context in a way that is non-breaking ie. something like
info.GoContext()
??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.
@choonkeat Would you be able to submit this Issue as a feature request? It's maybe something we could look to supporting.
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.
ya :-P
it was more of a comment, since idiomatic Go callback function usually comes with
context.Context
. But if it doesn't fit v8, I don't mind passing my owncontext.Context
via closurei'm still trying to wrap my head around how things "should" work. e.g. implementing
fetch
i'd need to return a Promise i think and thus will be tracking #61 closelyThere 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.
Yes, to implement true
fetch
you need to return the Promise; A poor man's version is here: https://github.com/rogchap/v8go/blob/master/function_template_test.go#L52There 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.
FYI. this is better supported now that we have #76