com-lihaoyi / scalatags

ScalaTags is a small XML/HTML construction library for Scala.
https://com-lihaoyi.github.io/scalatags/
MIT License
758 stars 117 forks source link

scalatags.JsDom.short._ contains no Tags #86

Closed stewSquared closed 8 years ago

stewSquared commented 9 years ago

According to the docs, import bundle.short._ "imports the contents of Tags and DataConverters, but aliases Attrs and Styles as *"

That's blatantly not the case; no tags are imported. This could be construed as a bug in the documentation, but I'm led to believe that it's a bug in the implementation, because further down in the docs, an example is given with tags like div and p being used after importing short._.

You can import tags from elsewhere, eg. import tags._, but you get compile errors for expressions like div(List(p,p)). More generally, if you import underscore from any two subclasses of scalatags.JsDom.Cap, then implicitly[List[Modifier] => Modifier] fails to compile.

  1. Basically, the only way to get something like:
    div(div.render, List(*.width:=300.px, p))

to compile is to:

import scalatags.JsDom.short.{*,bindJsAny,bindJsAnyLike,bindNode}
import scalatags.JsDom.tags._
import scalatags.JsDom.implicits._
import scalatags.DataConverters._

or import all._;, which kinda kills the point of importing the *.

In short, it's useless to import short._, which I don't believe was the intention.

stewSquared commented 8 years ago

Note that this is a one line change in each of https://github.com/lihaoyi/scalatags/blob/master/scalatags/js/src/main/scala/scalatags/JsDom.scala#L43 and https://github.com/lihaoyi/scalatags/blob/master/scalatags/shared/src/main/scala/scalatags/Text.scala#L38. I'll be happy to make the pull request adding with jsdom.Tags to object short. I just wanted to make sure we agree this is a bug.

To explain a bit more clearly why I think this is appropriate:

This compiles:

import scalatags.JsDom.all._
div(width:=10.px, List(p,p))

But you might not want all the styles and attributes polluting your namespaces, so short is provided. The div above would become div(*.width:=10.px, List(p,p)). However, contrary to the docs, this doesn't compile:

import scalatags.JsDom.short._
div(*.width:=10.px, List(p,p))

You could add import scalatags.JsDom.tags._, and it will still fail to compile.

It will compile if you import short.*; all._, or short._; tags.{p,div} but those are undesirable for obvious reasons. So you might decide to try:

scalatags.JsDom.short.*
scalatags.JsDom.tags._
div(*.width:=10.px, List(p,p))

That compiles - yay! But other things will break unless you also import:

import scalatags.JsDom.short.{bindJsAny,bindJsAnyLike,bindNode}
import scalatags.JsDom.implicits._
import scalatags.DataConverters._

and probably a bunch of other things that I'm missing and aren't meant to be part of the public API.

Same is true if you replace all occurences of JsDom with Text. In the case where you try to import text._ with short._, compile errors won't show up in the REPL unless you wrap all of those lines in an object{...}.

TL;DR

As per the docs, I want to write div(*.width:=10.px, List(p,p)) and can't without hideous imports. It's a two-line fix if we agree it's a bug.

stewSquared commented 8 years ago

So, after a bit more digging, I've found a bug in the tests for manual Imports. which is that import bundle.all._ is in scope, which kinda defeats the point of manual imports. I'll tweak the tests and offer a PR.

lihaoyi commented 8 years ago

Thanks for digging into this @stewSquared!!!

lihaoyi commented 8 years ago

Your fix has been merged