-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
Support useECMAScript2015 option in ScalaJSModule #1004
Conversation
Great! Thanks for this pull request.
I like the idea of running the tests without requiring some external tool (node). I think we are not necessarily forced to use the same classpath for test for different Scala.js versions. But I do not have that much Scala.js tooling experience, so it's probably easier to say that than to implement it. Do you have by any chance the other setup (which failed) around, so that I could play with it? |
Consider that Scala-js NodeJSEnv is depending from "node" too:
So if you use it or not you require package mill.scalajslib
import org.scalajs.jsenv.nodejs.NodeJSEnv
import org.scalajs.jsenv.Input
import org.scalajs.jsenv.test.kit.TestKit
import mill.scalajslib.api.ModuleKind
import scala.concurrent.duration._
object ScalaJsUtils {
def runJSExpectingOut(path: os.Path, moduleKind: ModuleKind, expectedOut: String): Unit = {
val input: org.scalajs.jsenv.Input = moduleKind match {
case ModuleKind.NoModule => new Input.Script(path.toNIO)
case ModuleKind.CommonJSModule => new Input.CommonJSModule(path.toNIO)
case ModuleKind.ESModule => new Input.ESModule(path.toNIO)
}
val jsEnv = new NodeJSEnv()
val testKit = new TestKit(jsEnv, 1.second)
testKit.withComRun(Seq(input)){ run =>
run.expectOut(expectedOut)
}
}
} And was requiring |
@lefou Could you try to rerun https://travis-ci.org/github/lihaoyi/mill/jobs/744146502 ? |
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.
LG from Scala.js POV.
def link(sources: Array[File], | ||
libraries: Array[File], | ||
dest: File, | ||
main: String, | ||
testBridgeInit: Boolean, // ignored in 0.6 | ||
fullOpt: Boolean, | ||
moduleKind: ModuleKind) = { | ||
moduleKind: ModuleKind, | ||
useECMAScript2015: Boolean /* ignored in 0.6 */) = { |
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.
This isn't ignored..
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.
So Scala.js 0.6.33 supports useECMAScript2015
?
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.
Yes, you're right! I'm using it 🙈
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.
Fixed ✅
- Support ESModule ModuleKind - Update Scala.js 0.6 to 0.6.33 - Update Scala.js 1 to 1.3.1 - Add useECMAScript2015 to test matrix just for last version - Update Scala versions it tests - Update ScalaJsUtils to use node.js instead of nashorn The initial idea was to use the NodeJSEnv from Scala.js to run the tests. But this created a problem since adding a particular NodeJSEnv to the classpath broke the other version of Scala.js linking. For example using NodeJSEnv from Scala.js 1.3.1 made fastOpt in Scala.js 0.6 break. So at the end the function is just calling "node" using `os.proc`
51999ea
to
ded0bac
Compare
Did a |
Travis job 2 is now always failing. Please ignore it for now. |
LGTM. I started another Travis Jobs just to be sure: https://travis-ci.org/github/lefou/mill/builds/744212036 |
Thank you, @lolgab ! |
The initial idea was to use the NodeJSEnv from Scala.js to run the
tests. But this created a problem since adding a particular NodeJSEnv
to the classpath broke the other version of Scala.js linking. For example
using NodeJSEnv from Scala.js 1.3.1 made fastOpt in Scala.js 0.6 break.
So at the end the function is just calling "node" using
os.proc