Add "engines" field to package.json

basham commented 4 years ago

Especially since this project uses experimental flags, it may be best to include an "engines" field to package.json, so we know what versions of Node are required to run WebRun.

RangerMauve commented 4 years ago

At the moment webrun actually bundles Node 10, but I'm not sure if newer versions support the necessary flags.

A PR with an engines field would be welcome. 😁

basham commented 4 years ago

Ah, I overlooked that it bundled Node. I'll look into this over this next week and report back.

basham commented 4 years ago

I reviewed the WebRun source code, and compared what you used with Node's docs. In summary, I see no reason why you can't just allow any version of Node v10.0.0 and above to work alongside of WebRun, rather than embedding a fixed version of Node like it currently does.

This should just require modifying package.json by removing node from devDependencies and adding this field somewhere:

{
  "engines": { "node": ">=10" }
}

APIs used in WebRun

`--experimental-repl-await`

Added in v10.0.0

`repl.start`

Added in v10.0.0

Note: There is a change in v12 concerning the terminal option you use, but I don't think it would negatively affect what you're doing.

The terminal option now follows the default description in all cases and useColors checks hasColors() if available.

basham commented 4 years ago

Another thing that I hadn't considered until I was working on the pull request (#31), is that the other dependencies in this project may have other Node needs. So, even though you may be safe with Node v10+, I haven't done the research to know how safe any of the other dependencies are with that choice.

RangerMauve / webrun