I think that it's more reasonable to have separate CSS and JS files. In many places we have Webpack configured to simply load CSS into JS files. While convenient, this caused us some problems. Namely, you delay loading JS as much as possible (usually, keep it close to </body>). This means that the <link> with CSS will be added very late to the <head> which gives you no control over the order of these stylesheets which is a problem if you want to override some styles.
Also, I'm unsure about how having CSS built into JS affects load times. That would need to be thoroughly researched so I'd feel safer if by default we proposed CSS and JS separately.
Is anyone against this? cc @fredck @oleq
Reinmar
All 13 comments
Sounds like a plan. How much work is it?
oleq
on 24 Aug 2017
10 minutes :D Plus updating documentation – 2h ;)
Reinmar
on 24 Aug 2017
We'd need also to instruct in the documentation how to avoid caching issues (e.g. in best practices) for users downloading zip packages or people loading files from node_modules, where the version number is not included in a path. In case of a single file with the editor the only risk is that someone will use outdated ckeditor.js. In case of multiple files the risk is that there will be ckeditor.js and css file mismatch.
wwalc
on 31 Aug 2017
You had to ruin the fun :P Where do you think we should mention this? I feel that wherever we'll put it, there will still be developers who will forget about the cache. It's not only CKE's problem and you're not able to document it everywhere.
Reinmar
on 31 Aug 2017
Performance wise, it is not ideal to have our CSS in the header.
CSS is "render blocking", so our CSS loading will block rendering the page, even if it is used only when editors are created. Therefore, it is ideal to have this CSS loaded later.
Other than that, it'll introduce a bit more complication for implementors, added by the issue identified by Wiktor.
Then regarding the possibility of overriding the styles, the order of the style elements is the one that rules this, even for CSS injected on the fly. This means that, CSS injected by CKEditor should go at the beginning of head not at the end of it.
The following shows exactly the above:
https://jsfiddle.net/ryv3abvj/
vs
https://jsfiddle.net/3t4u7awy/1/
So, ideally, I would leave CSS and js together, as we have already.
fredck
on 1 Sep 2017
@fredck Both fiddles look the same. I think something went wrong.
oleq
on 1 Sep 2017
If we decide to go with separate CSS file then we could come with some generic "Best practices" article for CKEditor 5 builds, where we'd list various hints for users, including this one. Let's see if we go with the separate CSS file first. Sorry :D
wwalc
on 1 Sep 2017
@oleq, there was some mess with JSFiddle... I've updated the links... can you check it again?
fredck
on 1 Sep 2017
Performance wise, it is not ideal to have our CSS in the header.
CSS is "render blocking", so our CSS loading will block rendering the page, even if it is used only when editors are created. Therefore, it is ideal to have this CSS loaded later.
I'm afraid that this isn't that simple. I'm not an expert myself, but e.g. if you have CSS files in the <head> from the start of when the browser parses the HTML it can start prefetching them as soon as possible. And they are all available at one point and the page can be rendered once.
If you have JS adding CSS later, the page needs to be re-rendered which is heavier for CPU (which is an important factor when talking about heavy pages or mobile devices). It's hard to guess here what kind of deoptimisation might happen if we do such things.
So, I wouldn't risk myself to have an opinion about that. It's an extremely complicated topic, a lot of process happening under the browsers' hoods. If we want to make an educated guess, we should research it heavily.
Other than that, it'll introduce a bit more complication for implementors, added by the issue identified by Wiktor.
That's an issue indeed. But it's a typical issue which all implementors have to deal every day when they are creating any websites.
This means that, CSS injected by CKEditor should go at the beginning of head not at the end of it.
That's a problem – it's not CKEditor injecting anything. It's Webpack's loader. We don't have control over that. Besides, I wouldn't even be sure that CKEditor's CSS should go to the top of the <head> because that's unusual and would reduce the specificity of CKEditor's styles (rules with the same specificity are sorted by their source order).
Anyway, I see that there are doubts regarding extracting CSS. I think that we need to make more research and check with the community. So, I'm postponing this ticket.
I pushed t/18 with the current state of my work, which is actually all that needs to be done here.
Reinmar
on 1 Sep 2017
If you have JS adding CSS later, the page needs to be re-rendered which is heavier for CPU
The point is privileging the download performance (which is much more impacting than re-rendering), so users will be able to see the page content earlier. We should not care about the browser rendering performance by itself, in this specific case.
I'm for checking about the possibility of having the Webpack loader pushing the CSS to the top of head.
fredck
on 1 Sep 2017
I'm for checking about the possibility of having the Webpack loader pushing the CSS to the top of head.
There's https://github.com/webpack-contrib/style-loader#insertat.
oleq
on 1 Sep 2017
OK, let's leave this topic for now. In 1.0.0 alpha we'll go with what we have currently (single file).
Reinmar
on 24 Sep 2017
We haven't got any reports about this so far. Perhaps simplicity has won and no one even thought about this. And people who do more complex integrations use their own Webpack setups. Besides, we have CSS extraction covered by the docs.
So, I'm closing this as invalid. Feel free to comment if you disagree.
Reinmar
on 14 Nov 2017
Related issues
wwalc
·
3Comments
MansoorJafari
·
3Comments
pjasiun
·
3Comments
MCMicS
·
3Comments
Reinmar
·
3Comments
Most helpful comment
There's https://github.com/webpack-contrib/style-loader#insertat.