eventloop: Allow creating loop without loading "console" module

[nwidger]

Remove the call to console.Enable in eventloop.NewEventLoop, thus making it possible to create an event loop without automatically loading the console commands into the runtime. The eventloop tests which make calls to console.log have been updated to call console.Enable within loop.Run just before evaluating the test's JavaScript string.

[dop251]

What about those who already rely on that?

[nwidger]

@dop251 I wondered about that, too. If we want to preserve backwards-compatibility with existing behavior, I think our only option would be to create a new NewEventLoopNoConsole constructor (or something like that) as there is no way to safely change NewEventLoop such that it can be told not to load console. If backwards-compatibility is important enough, I'd be happy to do something like that, but I do think the existing behavior is quite unfortunate. I was surprised when I removed my own call to console.Enable and found that the console commands where still loaded into the runtime because of my use of eventloop. As far as I can tell, the console commands are not necessary for the event loop to function correctly, and I can only assume it was added in to make writing the eventloop tests easier. For users who want strict control over what functionality is exposed to scripts evaluated in the runtime, the current situation is rather frustrating. But again, I completely understand not wanting to break backwards-compatibility without consideration.

[nwidger]

@dop251 One idea that would maintain backwards-compatibility would be to update NewEventLoop to use the "functional options" pattern. It might look something like this:

type EventLoop struct {
    vm            *goja.Runtime
    jobChan       chan func()
    jobCount      int32
    running       bool
    enableConsole bool
}

func NewEventLoop(opts ...option) *EventLoop {
    vm := goja.New()

    loop := &EventLoop{
        vm:            vm,
        jobChan:       make(chan func()),
        enableConsole: true,
    }

    for _, opt := range opts {
        opt(loop)
    }

    new(require.Registry).Enable(vm)
    if loop.enableConsole {
        console.Enable(vm)
    }
    vm.Set("setTimeout", loop.setTimeout)
    vm.Set("setInterval", loop.setInterval)
    vm.Set("clearTimeout", loop.clearTimeout)
    vm.Set("clearInterval", loop.clearInterval)

    return loop
}

type option func(*EventLoop)

func EnableConsole(enableConsole bool) option {
    return func(loop *EventLoop) {
        loop.enableConsole = enableConsole
    }
}

Thus, calling eventloop.NewEventLoop() would return an event loop that loads console, but eventloop.NewEventLoop(eventloop.EnableConsole(false)) would not. Would this be more acceptable? If so I'm happy to update this PR.

[dop251]

Yes, that works for me.

[nwidger]

@dop251 Sounds good, I've updated the PR to reflect the new strategy.