[wip] Transpile external css

[giuseppeg]

fixes #83

[giuseppeg]

@rauchg @nkzawa early feedbacks are appreciated, what do you think about the syntax? should we stick with the style tag instead?

[rauchg]

1. Isn't src better?
2. How would we keep track of the file for hot code reloading?

[giuseppeg]

Isn't src better?

I am going for <style jsx src="./styles.js" />

How would we keep track of the file for hot code reloading?

We don't have to it should happen automatically because the external stylesheet is a regular js module. Not sure if I am missing something though but I will able to tell soon – I am half way through implementing the import injection and style tag replacement :)

[giuseppeg]

need to think if this solution could work at all since the hash computation could not be possible with this approach

[giuseppeg]

need to fix this (and maybe call it makeImportExternal) because transpiled (external) stylesheets are exporting a single object (default)

[giuseppeg]

I thought a little bit about this approach and I think that it could work in case we are cool with allowing only a single style tag when using external css:

/* valid */

export default () => (
  <p>
      test
      <style jsx src="./style.js" />
  </p>
)

/* not valid */

export default () => (
  <p>
      test
      <style jsx>{`p { font-size: 3em }`}</style>
      <style jsx src="./style.js" />
  </p>
)

This is because I would use the resolved src path for hashing.

@nkzawa @rauchg if you like this solution and think that this feature could add value to the library I will finalize it and put in a mergeable state.

[nkzawa]

I think it can work even if there are multiple style elements since we already support with global ?

[nkzawa]

Does this syntax mean we can't import others from imported styles ? 🤔

[arunoda]

Why don't we simply for a template string like:

const foo = 'red'

export default `
  p { color: ${foo} }
`

[giuseppeg]

This would be the best api however is not that easy to achieve this because in the consumer file (the one that imports this) we need to hash `p { color: ${foo} }` and once we have the hash we need to transform the css (add the prefix to it etc).

[giuseppeg]

I think it can work even if there are multiple style elements since we already support with global ?

when external files are transpiled they don't have knowledge of the consumer component therefore when transforming the css we don't have all of the necessary information to create a proper hash (jsxId).

/* external file */

`p { color: red }`

The jsxId here is hash(`p { color: red }`) === 'woot'
i.e. p[data-jsx="woot"] { color: red }

/* consumer */

`p { font-size: 3em }` // own styles
src="./style.js"

The jsxId in this case will be hash(externalIdReference + `p { font-size: 3em }`) === 'wat'

i.e. p[data-jsx="wat"] { font-size: 3em }
and elements are scoped using wat: <p data-jsx="wat">test</p>
external styles won't apply because they have id woot instead.

Does this syntax mean we can't import others from imported styles ?

Uh this is kinda tricky, I am using that syntax so that I can tell whether a module is an external styled-jsx sheet. If we don't do so then we'd need a separate plugin to transpile external files.

[giuseppeg]

The only way I can think of to allow multiple style elements is to transpile external css like so:

p[data-jsx~="woot"] { color: red }

(notice the ~)

and the consumer eventually will be:

p[data-jsx~="wat"] { font-size: 3em }

and

<p data-jsx="woot wat">test</p>

[rauchg]

The other thing I've been thinking about is that we could have "flow" analysis. This would work:

import style from './style'
export default (
  <div>
    <p>hi</p>
    <style jsx>{style}</style>
  </div>
)

and this would work:

cons style = `
  p { color: red }
`
export default (
  <div>
    <p>hi</p>
    <style jsx>{style}</style>
  </div>
)

[giuseppeg]

🤔 I thought about case 1 for external files but in order to do so we'd need to hash using the resolved file name. Also when transpiling an external file we need a way to tell if that's just an external stylesheet or any other module/react component. That's why in my wip I export JSX (a style tag).

[rauchg]

Why can't we just hash based on contents? The idea is that by analyzing the "flow" we can access the raw string exactly the same as if it was inlined inside <style jsx>. Hence, IMHO, why the approach is elegant

[rauchg]

It also seems like a natural complement of the work we've been doing to allow constants inside the source

[tgoldenberg]

@rauchg based on .jsx syntax, my first reaction was to automatically assume that you could do this <style jsx>{style}</style>. The fact that it does not do this goes against basic React syntax conventions. Also, not being able to load multiple style tags seems counter-intuitive, as well. If we are going to break people's expectations, we should make it very clear in the beginning, in order to avoid unnecessary confusion. Will be really excited to see the solution in action, and great work!

[rauchg]

@tgoldenberg all of those problems are tractable. We tend to ship and iterate.

[rauchg]

Also, whenever we go against intuition, we attempt to provide very clear error messages: for example, if you're trying to use prop or state inside the style text

[giuseppeg]

Don't get me wrong I actually like the idea of flow analysis.

The idea is that by analyzing the "flow" we can access the raw string exactly the same as if it was inlined inside <style jsx>.

This means that we'd need to read (or eval) external files as we transpile and in general the traversal would get a bit trickier. Also we may need to revisit the expressions logic since at the moment we just replace them with placeholders before hashing (we don't analyse them).

In the end though probably this will result in the better api.

[giuseppeg]

as I said maybe for external files we can hash the resolved file path instead of the content (css)