robinweser / fela

State-Driven Styling in JavaScript
http://fela.js.org
MIT License
2.27k stars 182 forks source link

Error when build #191

Closed steida closed 7 years ago

steida commented 7 years ago

Not sure why, but compiled Este has this error:

screen shot 2017-01-28 at 09 41 29

steida commented 7 years ago

When placeholderPrefixer() is commented, it works.

robinweser commented 7 years ago

I only tested this in development (where it doesn't use insertRule) Looks like CSSStyleSheet does not allow ::-moz-placeholder. We need to use textContent here I guess. See: https://github.com/rtsao/styletron/issues/69#issuecomment-271966177

steida commented 7 years ago

Correction, it doesn't work even without placeholderPrefixer()

screen shot 2017-01-29 at 05 46 35

steida commented 7 years ago

Working on fix. All we need it dynamically testing which selector is supported.

steida commented 7 years ago

Hmm, we need to render everything on the server, and do try catch detection only on client.

steida commented 7 years ago

This is temp fix https://github.com/este/este/commit/f1d25590e363eb6b7822295ad3318e2a898ccba8#diff-708d62a603a169ef52169d279ec05623

steida commented 7 years ago

How you test dev Fela with real project? npm link? Can you use my diff to fix? It's pretty simple.

steida commented 7 years ago

@rofrischmann Can you open it please? This should really be fixed only for Chrome.

steida commented 7 years ago

Hmm, the fix I suggested breaks things. screen shot 2017-02-05 at 20 55 01

abelaska commented 7 years ago

@steida exactly.

Fela 4.2.1 with fela-plugin-placeholder-prefixer 4.2.1 works just fine for me. Your workaround este/este@f1d2559#diff-708d62a603a169ef52169d279ec05623 is no longer needed.

steida commented 7 years ago

@abelaska I know, it's removed in branch already. Fela fix was released after talk with Robin.

abelaska commented 7 years ago

@steida ok