japgolly / scalacss

Super type-safe CSS for Scala and Scala.JS.
https://japgolly.github.io/scalacss/book
Apache License 2.0
338 stars 44 forks source link

Problem with generated classnames when more than 26 inline stylesheets are added to document #122

Closed vmarc closed 7 years ago

vmarc commented 7 years ago

First: thank you very much for this fantastic library!

In my application, all components that require component specific styling include an object that extends "StyleSheet.Inline" inside the object that contains the component definition. The addToDocument() method is used to add the styles for each of these components to the document.

There is a problem when there are more than 26 components with an inline stylesheet.

When using ProdDefaults, the classnames that are generated for component 27, 28, 29 and 30 are "{0", "|0", "}0" and "~0" respectively. No styling is done for these classnames by the browser (invalid characters?). I tried Chrome and Firefox.

The problem is illustrated by the project code in the zipfile (unzip, open sbt, run fastOptJS and open "index.html" in your browser). The screenshot below shows the resulting page. Note that the styling of components 27, 28, 29, 30 and 31 is missing (no bar).

scala-css-issue.zip

issue

I suspect the problem is somewhere around NameGen.short (perhaps nextShortPrefix?).

Questions:

1) Is the approach with adding a separate inline stylesheet per component that requires styling as intended, or should I have used a different pattern? 2) Should I perhaps try to setup a production configuration with a custom NameGen that for example starts using a two character prefix and an extra separator starting with stylesheet 27 (see below)? Or can the standard NameGen.short be adapted?

Regards, Marc


Example alternative classnames: 1 _a0 or _a_0 2 _b0 or _b_0 3 _c0 or _c_0 ... 26 _z0 or _z_0 27 _ab_0 28 _ac_0 ... 52 _az_0 53 _ba_0 54 _bb_0 ...

japgolly commented 7 years ago

Hi @vmarc , firstly thanks for the nice feedback. I'm glad you enjoy it!

I don't have a chance today to look at your sample code (awesome and thanks for providing it btw!). But I did have time to inspect the CSS of one of the pages of my work project which uses ScalaCSS with > 26 styles.

In my case I'm using scalacss.devOrProdDefaults and it's working as expected, it goes from ._b0, ._b1 ... _b9 _ba ... _bz _b- b then rolls over to _b00 _b01 etc. Full CSS here: https://gist.github.com/japgolly/20c05ad864223909dbff6d057830bbba

It definitely shouldn't be using "|" and "{" in classnames. I wonder why it isn't in my project?

an-tex commented 7 years ago

Hi, I've just today experienced the same issue after adding new inline stylesheets. Classnames are like {8 {9

an-tex commented 7 years ago

As far as I see this function is responsible for generating the prefixes in production: scalacss.internal.mutable.Register.NameGen.short()

Calling it multiple times, e.g: (1 to 30).map(_ => scalacss.internal.mutable.Register.NameGen.short().next(ClassNameHint("moin"))._1.value) Outputs: Vector(_a0, _b0, _c0, _d0, _e0, _f0, _g0, _h0, _i0, _j0, _k0, _l0, _m0, _n0, _o0, _p0, _q0, _r0, _s0, _t0, _u0, _v0, _w0, _x0, _y0, _z0, _{0, _|0, _}0, _~0)

I guess the problem is with: scalacss.internal.mutable.Register.NameGen.nextShortPrefix()

Internally it's using ('a' + _nextShortPrefix).toChar Where _nextShortPrefix is a growing integer, but from Int 26 on it's already not alphanumeric any more... E.g: (1 to 30).map(_ => scalacss.internal.mutable.Register.NameGen.nextShortPrefix()) returns Vector(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, {, |, }, ~)

I'm on 0.5.3 btw but seems code is identical to master.

an-tex commented 7 years ago

I have a workaround matching your second point @vmarc

  val CssSettings =
    if (Platform.DevMode) DevDefaults
    else new Prod with Exports {
      private[this] var _nextShortPrefix = 0

      def nextShortPrefix(): String = {
        val p = toAlphabet(_nextShortPrefix)
        _nextShortPrefix += 1
        p
      }

      def toAlphabet(i: Int): String = {
        val quot = i / 26
        val rem = i % 26
        val letter = ('a' + rem).toChar
        if (quot == 0) letter.toString
        else toAlphabet(quot - 1) + letter
      }

      override def cssRegisterNameGen = NameGen.short(prefix2 = nextShortPrefix())
    }

The key is the toAlphabet function, which for

(0 to 200).map(toAlphabetic)

returns

Vector(a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z, aa, ab, ac, ad, ae, af, ag, ah, ai, aj, ak, al, am, an, ao, ap, aq, ar, as, at, au, av, aw, ax, ay, az, ba, bb, bc, bd, be, bf, bg, bh, bi, bj, bk, bl, bm, bn, bo, bp, bq, br, bs, bt, bu, bv, bw, bx, by, bz, ca, cb, cc, cd, ce, cf, cg, ch, ci, cj, ck, cl, cm, cn, co, cp, cq, cr, cs, ct, cu, cv, cw, cx, cy, cz, da, db, dc, dd, de, df, dg, dh, di, dj, dk, dl, dm, dn, do, dp, dq, dr, ds, dt, du, dv, dw, dx, dy, dz, ea, eb, ec, ed, ee, ef, eg, eh, ei, ej, ek, el, em, en, eo, ep, eq, er, es, et, eu, ev, ew, ex, ey, ez, fa, fb, fc, fd, fe, ff, fg, fh, fi, fj, fk, fl, fm, fn, fo, fp, fq, fr, fs, ft, fu, fv, fw, fx, fy, fz, ga, gb, gc, gd, ge, gf, gg, gh, gi, gj, gk, gl, gm, gn, go, gp, gq, gr, gs, gt, gu, gv, gw, gx, gy, gz, ha, hb, hc, hd, he, hf, hg, hh, hi, hj, hk, hl, hm, hn, ho, hp, hq, hr, hs, ht, hu, hv, hw, hx, hy, hz, ia, ib, ic, id, ie, if, ig, ih, ii, ij, ik, il, im, in, io, ip, iq, ir, is, it, iu, iv, iw, ix, iy, iz, ja, jb, jc, jd, je, jf, jg, jh, ji, jj, jk, jl, jm, jn, jo, jp, jq, jr, js, jt, ju, jv, jw, jx, jy, jz, ka, kb, kc, kd, ke, kf, kg, kh, ki, kj, kk, kl, km, kn, ko, kp, kq, kr, ks, kt, ku, kv, kw, kx, ky, kz, la, lb, lc, ld, le, lf, lg, lh, li, lj, lk, ll, lm, ln, lo, lp, lq, lr, ls, lt, lu, lv, lw, lx, ly, lz, ma, mb, mc, md, me, mf, mg, mh, mi, mj, mk, ml, mm, mn, mo, mp, mq, mr, ms, mt, mu, mv, mw, mx, my, mz, na, nb, nc, nd, ne, nf, ng, nh, ni, nj, nk, nl, nm, nn, no, np, nq, nr, ns, nt, nu, nv, nw, nx, ny, nz, oa, ob, oc, od, oe, of, og, oh, oi, oj, ok, ol, om, on, oo, op, oq, or, os, ot, ou, ov, ow, ox, oy, oz, pa, pb, pc, pd, pe, pf, pg, ph, pi, pj, pk, pl, pm, pn, po, pp, pq, pr, ps, pt, pu, pv, pw, px, py, pz, qa, qb, qc, qd, qe, qf, qg, qh, qi, qj, qk, ql, qm, qn, qo, qp, qq, qr, qs, qt, qu, qv, qw, qx, qy, qz, ra, rb, rc, rd, re, rf, rg, rh, ri, rj, rk, rl, rm, rn, ro, rp, rq, rr, rs, rt, ru, rv, rw, rx, ry, rz, sa, sb, sc, sd, se, sf, sg, sh, si, sj, sk, sl, sm, sn, so, sp, sq, sr, ss, st, su, sv, sw, sx, sy, sz, ta, tb, tc, td, te, tf, tg, th, ti, tj, tk, tl, tm, tn, to, tp, tq, tr, ts, tt, tu, tv, tw, tx, ty, tz, ua, ub, uc, ud, ue, uf, ug, uh, ui, uj, uk, ul, um, un, uo, up, uq, ur, us, ut, uu, uv, uw, ux, uy, uz, va, vb, vc, vd, ve, vf, vg, vh, vi, vj, vk, vl, vm, vn, vo, vp, vq, vr, vs, vt, vu, vv, vw, vx, vy, vz, wa, wb, wc, wd, we, wf, wg, wh, wi, wj, wk, wl, wm, wn, wo, wp, wq, wr, ws, wt, wu, wv, ww, wx, wy, wz, xa, xb, xc, xd, xe, xf, xg, xh, xi, xj, xk, xl, xm, xn, xo, xp, xq, xr, xs, xt, xu, xv, xw, xx, xy, xz, ya, yb, yc, yd, ye, yf, yg, yh, yi, yj, yk, yl, ym, yn, yo, yp, yq, yr, ys, yt, yu, yv, yw, yx, yy, yz, za, zb, zc, zd, ze, zf, zg, zh, zi, zj, zk, zl, zm, zn, zo, zp, zq, zr, zs, zt, zu, zv, zw, zx, zy, zz, aaa, aab, aac, aad, aae, aaf, aag, aah, aai, aaj, aak, aal, aam, aan, aao, aap, aaq, aar, aas, aat, aau, aav, aaw, aax, aay, aaz, aba, abb, abc, abd, abe, abf, abg, abh, abi, abj, abk, abl, abm, abn, abo, abp, abq, abr, abs, abt, abu, abv, abw, abx, aby, abz, aca, acb, acc, acd, ace, acf, acg, ach, aci, acj, ack, acl, acm, acn, aco, acp, acq, acr, acs, act, acu, acv, acw, acx, acy, acz, ada, adb, adc, add, ade, adf, adg, adh, adi, adj, adk, adl, adm, adn, ado, adp, adq, adr, ads, adt, adu, adv, adw, adx, ady, adz, aea, aeb, aec, aed, aee, aef, aeg, aeh, aei, aej, aek, ael, aem, aen, aeo, aep, aeq, aer, aes, aet, aeu, aev, aew, aex, aey, aez, afa, afb, afc, afd, afe, aff, afg, afh, afi, afj, afk, afl, afm, afn, afo, afp, afq, afr, afs, aft, afu, afv, afw, afx, afy, afz, aga, agb, agc, agd, age, agf, agg, agh, agi, agj, agk, agl, agm, agn, ago, agp, agq, agr, ags, agt, agu, agv, agw, agx, agy, agz, aha, ahb, ahc, ahd, ahe, ahf, ahg, ahh, ahi, ahj, ahk, ahl, ahm, ahn, aho, ahp, ahq, ahr, ahs, aht, ahu, ahv, ahw, ahx, ahy, ahz, aia, aib, aic, aid, aie, aif, aig, aih, aii, aij, aik, ail, aim, ain, aio, aip, aiq, air, ais, ait, aiu, aiv, aiw, aix, aiy, aiz, aja, ajb, ajc, ajd, aje, ajf, ajg, ajh, aji, ajj, ajk, ajl, ajm, ajn, ajo, ajp, ajq, ajr, ajs, ajt, aju, ajv, ajw, ajx, ajy, ajz, aka, akb, akc, akd, ake, akf, akg, akh, aki, akj, akk, akl, akm, akn, ako, akp, akq, akr, aks, akt, aku, akv, akw, akx, aky, akz, ala, alb, alc, ald, ale, alf, alg, alh, ali, alj, alk, all, alm)

If that looks fine to you @japgolly I can create a PR with an adapted _nextShortPrefix function.

Cheers

japgolly commented 7 years ago

Thanks for digging in to that. I'm still really curious as to why I'm not experiencing this issue myself but I don't want to let my lack of time block this fix for anyone else. So yes please @an-tex , a PR would be appreciated and then I'll get a new release out ASAP.

an-tex commented 7 years ago

While implementing and testing a fix I figured out why you didn't see the issue @japgolly . It's because the initial description was inaccurate, as the issue arises not with more than 26 styles within one StyleSheet.Inline, but if more than 26 StyleSheet.Inline are registered...

Anyway here's the PR https://github.com/japgolly/scalacss/pull/125

japgolly commented 7 years ago

Ah, that makes sense. Thanks, merged!