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

Allow for importing styles #83

Closed
thisbejim opened this issue Jan 20, 2017 · 18 comments · Fixed by #148
Closed

Allow for importing styles #83

thisbejim opened this issue Jan 20, 2017 · 18 comments · Fixed by #148

Comments

@thisbejim
Copy link

Sometimes styles can be quite long and it makes sense for them to live in a seperate file. For example, if you are using next.js and you want to put your styles in a ./styles.js file next to a component (minimal styling just for the example):

export default `
  p {
    color: red;
  }
`

Then import it:

import styles from './styles';

export default () => (
  <div>
    <p>only this paragraph will get the style :)</p>
    <style jsx>{styles}</style>
  </div>
)

Will give you the following error: Module build failed: SyntaxError: Expected a template literal or String literal as the child of the JSX Style tag (eg: <style jsx>{some css}</style>), but got Identifier.

@giuseppeg
Copy link
Collaborator

I believe that external styles could be enabled with a <link jsx href="path/to/styles.js" /> tag or if we don't care about semantics <style jsx href="path/to/styles.js" />.

path/to/styles.js should export a styles literal:

export default `
  p { color: red; }
`

which we transpile to:

module.exports = {
  css: `p[data-jsx="woot"] { color: red; }`,
  id: 'woot'
}

Tbd how to handle the global flag.

@thysultan
Copy link
Contributor

@giuseppeg stylis does have support for handling sass like @import tokens, and passes it to a middleware if provided, you could use this to then import the requested files and return them as a string and the compiler will continue from there.

@giuseppeg
Copy link
Collaborator

@thysultan interesting... that works only for plain css though right? Now we support expressions within template literals so external styled jsx files should be javascript modules.

@thysultan
Copy link
Contributor

thysultan commented Jan 25, 2017

@giuseppeg Yes a string so you might do...

var m = new module.constructor();
m._compile('module.exports = `'+fs.readFileSync(filename, 'utf8')+'`', filename);
var str = m.exports;
return str;

or maybe even...

try {
    new Function("`"+fs.readFileSync(filename, 'utf8')+"`", arguments);
} catch(e) {
}

or even parse the file for template literals expressions from the middleware with regex and replace them with their eval'd result.

Notes no how middleware work: https://github.com/thysultan/stylis.js#middleware

@giuseppeg
Copy link
Collaborator

giuseppeg commented Jan 25, 2017

@thysultan the problem is that in styled jsx you could do the following

import {primary} from '../theme/colors'

export default `
  p { color: ${primary}; }
`

@thysultan
Copy link
Contributor

thysultan commented Jan 25, 2017

@giuseppeg couldn't you still transpile that with babel and do m._compile('module.exports = .... pointing to the content of the exported template literal.

@giuseppeg
Copy link
Collaborator

do you mean resolve the template literal to static string?

@thysultan
Copy link
Contributor

thysultan commented Jan 25, 2017

Yes, have babel compile.

import {primary} from '../theme/colors'

export default `
  p { color: ${primary}; }
`

Then pass that through

var m = new module.constructor();
m._compile('module.exports = `'+transpiledFileContent+'`', filename);
var str = m.exports;

Now you have a static string like "p { color: blue; }" then stylis can replace the import statement with this and parse/compile it. The same can be done for other file types i.e where as @import "foo.js" or something like @import "foo.sjsx" would import require a babel compile @import "foo.scss" or @import "foo" assumed non js files would not, stylis avoids @import "file.css" imports to avoid css syntax conflicts.

@davibe
Copy link

davibe commented Jan 26, 2017

I have used babel inline-import plugin https://www.npmjs.com/package/babel-plugin-inline-import
and registered it for .css file extention. I was able to do the same also using other plugins like https://github.com/gnandretta/babel-plugin-transform-css-import-to-string

With that babel automatically inlined my css in js. However, inlining file A in file B any changes in file A will not trigger hot-reloading unless you also change B.

I believe being able to use external files with hot-reloading is needed because

  • when styles get too long it's easyer to manage an external file. I know styles should not get too long expecially in components but global styles or typhograpy do. Those are not going away for large applications.
  • syntax hilight: right now only atom can do it. If you use another editor/tool you need to write a css plugin that is able to hilight css-in-js. It also means that you have to do that for every language you can possibly inline (for example i used markdown-in-js but there is also sass/less/..), for every editor/tool, you need to add support for hilighting by creating plugins.
  • in general it would allow people to use any tools they already use to generate the files they want to inline automatically.

@giuseppeg
Copy link
Collaborator

@davibe cool indeed using something like babel-plugin-inline-import gets the job done if you don't need constants in your jsx (string interpolation). Hot-reloading is out of the scope of this package.

@arunoda
Copy link
Contributor

arunoda commented Jan 29, 2017

Guys, I hacked for something like this:

export default class {
  render () {
    return (
      <div>
        <p>test</p>
        <style jsx src="./src-style.css" />
      </div>
    )
  }
}

See Implementation: https://github.com/arunoda/styled-jsx/blob/style-src/test/fixtures/src.js

But with this, we can't get hot reload for CSS changes. Which is not something great. So, I gave up on this idea.

@giuseppeg
Copy link
Collaborator

giuseppeg commented Jan 30, 2017

I believe that if we use js modules like i suggested above we would also get hot reload.

I am not a big fan of inlining css because potentially it could lead to duplicate code – probably not a problem with gzipped content but I'd happily avoid that. Also now we support constants so external css file should be js modules imho.

@sheerun
Copy link
Contributor

sheerun commented Mar 19, 2017

I'd imagine a nice approach would be to support @import keyword along composes, as so:

<div>
  <button>Hello world</button>
  <style jsx>{`
    @import 'bootstrap.css';

    button {
      composes: btn btn-primary;
      font-size: 30px;
    }
  `}</style>
</div>

@sheerun
Copy link
Contributor

sheerun commented Mar 19, 2017

@giuseppeg I've asked babel team for tips how to implement such plugin: babel/babel#5505

@sheerun
Copy link
Contributor

sheerun commented Mar 19, 2017

It seems changing parser per-file is totally possible:

https://github.com/lightscript/babel-plugin-lightscript/blob/master/src/index.js#L1001

@giuseppeg
Copy link
Collaborator

giuseppeg commented Mar 19, 2017

@sheerun thanks that's very interesting and useful information!

We are trying to keep CSS (styled-jsx) as standard compliant as possible without introducing custom features. The reason for this choice is that everybody has personal preferences and not necessarily universally wanted sets of custom features that would like to use/have and adding all of them would make the plugin very complex.

Those imho belong to user land and can be implemented as an extension of styled-jsx in some cases (see styled-jsx-postcss).

In this particular case I would like to avoid to reinvent the CSS Modules composes feature.

@yn5
Copy link

yn5 commented May 5, 2017

Any updates on this? Not being able to import styles is a big deal breaker for us, apart from that we'd love to use this library.

@giuseppeg
Copy link
Collaborator

@yn5 it is wip #148

giuseppeg added a commit that referenced this issue May 13, 2017
Styles can be defined in separate JavaScript modules and exported as strings. Fixes #83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants