From ad80fcc394a749b382dfe545db097aac36022d35 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Fri, 1 May 2020 14:26:58 +0300 Subject: [PATCH 1/2] fix setting js options on the wrong object previos to this we were getting a possibly unexported variable "options" and setting values to it. But the exported options could not have a local variable with the same name ... This also fixes a test to specifically need this to work. --- js/bundle.go | 2 +- js/runner_test.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/js/bundle.go b/js/bundle.go index b9d69810246..890c452e1fc 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -249,7 +249,7 @@ func (b *Bundle) Instantiate() (bi *BundleInstance, instErr error) { bi.exports[k] = fn } - jsOptions := rt.Get("options") + jsOptions := exports.Get("options") var jsOptionsObj *goja.Object if jsOptions == nil || goja.IsNull(jsOptions) || goja.IsUndefined(jsOptions) { jsOptionsObj = rt.NewObject() diff --git a/js/runner_test.go b/js/runner_test.go index 6f9f4a70857..ebc2718da37 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -179,19 +179,19 @@ func TestOptionsSettingToScript(t *testing.T) { func TestOptionsPropagationToScript(t *testing.T) { t.Parallel() data := ` - var options = { setupTimeout: "1s", myOption: "test" }; - exports.options = options; + exports.options = { setupTimeout: "1s", myOption: "test" }; exports.default = function() { - if (options.external) { + if (exports.options.external) { throw new Error("Unexpected property external!"); } - if (options.myOption != "test") { - throw new Error("expected myOption to remain unchanged but it was '" + options.myOption + "'"); + if (exports.options.myOption != "test") { + throw new Error("expected myOption to remain unchanged but it was '" + exports.options.myOption + "'"); } - if (options.setupTimeout != __ENV.expectedSetupTimeout) { - throw new Error("expected setupTimeout to be " + __ENV.expectedSetupTimeout + " but it was " + options.setupTimeout); + if (exports.options.setupTimeout != __ENV.expectedSetupTimeout) { + throw new Error("expected setupTimeout to be " + __ENV.expectedSetupTimeout + " but it was " + exports.options.setupTimeout); } - };` + }; + ` expScriptOptions := lib.Options{SetupTimeout: types.NullDurationFrom(1 * time.Second)} r1, err := getSimpleRunner("/script.js", data, From 35784c2498a14a06565b5d1e87ef05f721820675 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 4 May 2020 18:37:06 +0300 Subject: [PATCH 2/2] Also set "options" on the "exports" not just in the runtime --- js/bundle.go | 4 +++- js/runner_test.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/js/bundle.go b/js/bundle.go index 890c452e1fc..a2109581215 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -253,7 +253,9 @@ func (b *Bundle) Instantiate() (bi *BundleInstance, instErr error) { var jsOptionsObj *goja.Object if jsOptions == nil || goja.IsNull(jsOptions) || goja.IsUndefined(jsOptions) { jsOptionsObj = rt.NewObject() - rt.Set("options", jsOptionsObj) + if err := exports.Set("options", jsOptionsObj); err != nil { + return nil, err + } } else { jsOptionsObj = jsOptions.ToObject(rt) } diff --git a/js/runner_test.go b/js/runner_test.go index ebc2718da37..3f44957b104 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -147,6 +147,7 @@ func TestOptionsSettingToScript(t *testing.T) { t.Run(fmt.Sprintf("Variant#%d", i), func(t *testing.T) { t.Parallel() data := variant + ` + exports.options = options; exports.default = function() { if (!options) { throw new Error("Expected options to be defined!");