Skip to content
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

Build issues with scala 2.12.x in latest master #182

Closed
an-tex opened this issue Jul 14, 2020 · 4 comments · Fixed by #183
Closed

Build issues with scala 2.12.x in latest master #182

an-tex opened this issue Jul 14, 2020 · 4 comments · Fixed by #183

Comments

@an-tex
Copy link
Contributor

an-tex commented Jul 14, 2020

Phew I finally figured it out why I get those strange compilation errors:

[Error] [...]/target/streams/_global/stImport/_global/streams/sources/r/react/src/main/scala/typings/react/components/webview.scala:455: type mismatch;
[error]  found   : Builder.this.type (with underlying type typings.react.components.webview.Builder)
[error]  required: AnyRef
[error] Note that Builder extends Any, not AnyRef.
[error] Such types can participate in value classes, but instances
[error] cannot appear in singleton types or in reference comparisons.

It happens only with scala 2.12.x (tested with 2.12.11 and 2.12.12). 2.13.2 is fine.

I guess 2.12.x should still be supported? If yes, might it make sense to crossCompile the demos?

It can be reproduced in https://github.com/ScalablyTyped/ScalaJsReactDemos by changing the scalaVersion

@oyvindberg
Copy link
Collaborator

Aha! Thank you, that does narrow it down a lot!

@oyvindberg
Copy link
Collaborator

So Scala 2.12 just doesn't seem capable of expressing the combination of universal traits and this.type which is used.

This is a workaround we could do:

diff --git a/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/StBuildingComponent.scala b/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/StBuildingComponent.scala
index b369ae1de..5d91adedd 100644
--- a/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/StBuildingComponent.scala
+++ b/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/StBuildingComponent.scala
@@ -9,21 +9,22 @@ import scala.scalajs.js
 import scala.scalajs.js.`|`
 import scala.scalajs.js.annotation._
 
-trait StBuildingComponent[E, R <: js.Object] extends Any {
+trait StBuildingComponent[E, R <: js.Object, Self] extends Any {
+
   @scala.inline
   def args: js.Array[js.Any]
   @scala.inline
-  def set(key: String, value: js.Any): this.type = {
+  def set(key: String, value: js.Any): Self = {
     args(1).asInstanceOf[js.Dynamic].updateDynamic(key)(value)
-    this
+    this.asInstanceOf[Self]
   }
   @scala.inline
-  def withComponent(f: js.Any => js.Any): this.type = {
+  def withComponent(f: js.Any => js.Any): Self = {
     args.update(0, f(args(0)))
-    this
+    this.asInstanceOf[Self]
   }
   @scala.inline
-  def apply(mods: TagMod[E]*): this.type = {
+  def apply(mods: TagMod[E]*): Self = {
     mods.foreach((mod: TagMod[E]) => if (mod.isInstanceOf[AttrPair[_]]) {
       val a = mod.asInstanceOf[AttrPair[_]]
       set(a.name, a.value)
@@ -31,14 +32,14 @@ trait StBuildingComponent[E, R <: js.Object] extends Any {
       val o = mod.asInstanceOf[OptionalAttrPair[_]]
       if (o.value.isDefined) set(o.name, o.value.get)
   } else args.push(mod))
-    this
+    this.asInstanceOf[Self]
   }
   @scala.inline
-  def withKey(key: String): this.type = set("key", key)
+  def withKey(key: String): Self = set("key", key)
   @scala.inline
-  def withRef(ref: R => Unit): this.type = set("ref", ref)
+  def withRef(ref: R => Unit): Self = set("ref", ref)
   @scala.inline
-  def withRef(ref: ReactRef[R]): this.type = set("ref", ref)
+  def withRef(ref: ReactRef[R]): Self = set("ref", ref)
 }
 
 object StBuildingComponent {
@@ -50,7 +51,7 @@ object StBuildingComponent {
   }
   
   @scala.inline
-  implicit def make[E, R <: js.Object](comp: StBuildingComponent[E, R]): ReactElement = {
+  implicit def make[E, R <: js.Object, Self](comp: StBuildingComponent[E, R, Self]): ReactElement = {
     if (!scala.scalajs.runtime.linkingInfo.productionMode) {
       if (comp.args(0) == null) throw new IllegalStateException("This component has already been built into a ReactElement, and cannot be reused")
   }
@@ -62,7 +63,7 @@ object StBuildingComponent {
   }
   class Default[E, R <: js.Object] (val args: js.Array[js.Any])
     extends AnyVal
-       with StBuildingComponent[E, R]
+       with StBuildingComponent[E, R, Default[E, R]]
   
 }
 
diff --git a/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/react/components/abbr.scala b/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/react/components/abbr.scala
index 476a5d422..74b729c01 100644
--- a/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/react/components/abbr.scala
+++ b/tests/react-transition-group/check-slinky/r/react/src/main/scala/typingsSlinky/react/components/abbr.scala
@@ -15,11 +15,11 @@ object abbr {
   @scala.inline
   class Builder (val args: js.Array[js.Any])
     extends AnyVal
-       with StBuildingComponent[tag.type, HTMLElement with js.Object] {
+       with StBuildingComponent[tag.type, HTMLElement with js.Object, Builder] {
     @scala.inline
-    def dangerouslySetInnerHTML(value: Html): this.type = set("dangerouslySetInnerHTML", value.asInstanceOf[js.Any])
+    def dangerouslySetInnerHTML(value: Html): Builder = set("dangerouslySetInnerHTML", value.asInstanceOf[js.Any])
     @scala.inline
-    def defaultChecked(value: Boolean): this.type = set("defaultChecked", value.asInstanceOf[js.Any])
+    def defaultChecked(value: Boolean): Builder = set("defaultChecked", value.asInstanceOf[js.Any])
   }
   
   def withProps(p: DetailedHTMLProps[HTMLAttributes[HTMLElement], HTMLElement]): Builder = new Builder(js.Array(this.component, p.asInstanceOf[js.Any]))

@an-tex
Copy link
Contributor Author

an-tex commented Jul 15, 2020

Got it. I don't see any issues with the workaround. Good onya for finding it so quickly!

oyvindberg added a commit that referenced this issue Jul 16, 2020
@oyvindberg
Copy link
Collaborator

Ok, that workaround was much too heavy, both on the converter side and on the generated code side.

I'm rather deactivating extends AnyVal when building on 2.12. The consequence will be that the generated javascript code is a bit slower (it'll allocate every builder once each time it's used), but that will be good motivation to upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants