Jss: Add sheet rendering priority feature

[nathanmarks]

@kof quoting your reply from the stylishly issue:

@cvle lets move priority discussion to a separate issue in jss project, because this thread should be focused. I think I could add something to jss to allow manual priority definition.

Do you have an implementation in mind already?

I know previously you weren't 100% certain about adding this feature, but given what you said in the stylishly thread I wanted to start up a discussion about it here as it is a critical feature for us. Otherwise I will have to create + manage DOM nodes myself to pass to jss to use.

[cvle]

Same here, I could kick out all the custom renderer code from my theming prototype, when we have this in core.

All I need is to associate a priority number to the stylesheets, e.g. as an additional option. The renderer should mount stylesheets with lower priorities before stylesheets with higher priorities. If a new stylesheet has the same priority as an already mounted one, the renderer should append to the last stylesheet with the same priority. The latter is not a hard requirement but we should define the behavior when stylesheets have the same priority.

I think this is related to https://github.com/cssinjs/jss/issues/287.

[nathanmarks]

@kof

Since the DomRenderer is designed to have a new instance per StyleSheet, I see 2 options here for getting the insertion point in attach() for new sheets:

1) Use the sheets in options.jss.sheets.registry
2) Query the DOM in every attach() to get the style elements (assuming the priority would also be put in a data-xxx attribute)

[nathanmarks]

@cvle I made a rough priority feature: https://github.com/nathanmarks/jss/blob/priority/tests/integration/priority.js

[kof]

@nathanmarks what about this:

1. by default, all sheets have same priority, it is an option and can be passed
2. first style tag is rendered before all other style/link tags in the head
3. all next style tags are rendered after already rendered jss style tags in the order they are requested
4. if they have priority defined, renderer needs to consider priority and move sheets in the dom as they come, to fulfill the priority

Any issues or missing features here?

[nathanmarks]

@kof That should do the trick 👍

Quick question: what will be the allowed input for priority?

[kof]

Quick question: what will be the allowed input for priority?

You mean which option name would we want to use? e.g. "index", or "priority" or "order"

[nathanmarks]

@kof As in, can it be any number above 0?

[kof]

I would say it can be even any number, similar to z-index ...

[nathanmarks]

@kof Yeah, that works great 👍

I have tested using some code with this feature in jss. It's extremely powerful and makes it very easy to flatten selectors that were previously descendent or chained selectors.

[kof]

One feedback I have got from @b2whats on gitter is: people who are overwriting greedy global selectors from regular css in jss, will loose the ability to have a stronger selector from jss. Which means that there will be a breaking change if we don't introduce any optional way to have the old behavior.

[nathanmarks]

@kof providing some sort of rendering options using the current behaviour as default would work.

[kof]

Yeah, the question what is a better default, what we have now, jss sheets at the end, or the new one ....

[nathanmarks]

@kof If you don't want to release a breaking change, the current behaviour as the default. Otherwise, hmm... difficult question as there are a variety of use cases. Will have to think about it some more.

[kof]

I think breaking change is ok if it justifies the goal. Also it can be a global setup for a jss instance, so upgrading is just one option away.

[nathanmarks]

What do you think to also being able to set a "start position" for jss by placing <style data-jss></style> in the document beforehand at a chosen place between tags in the head.

I would say the default should be the current due to the fact that if you are developing something new using jss, there's no reason why you should be loading other CSS afterwards that needs to take priority. My hunch (I have no numbers to back this up) would be that in most cases you'd want your jss sheets to override something like a normalize.css reset. And third party marketing tools etc that inject CSS generally don't give a shit and insert things wherever they want 😃

For material-ui, we always want users sheets such as css modules to load after. But our use case is not representative of most jss users.

Side note: Do you see yourself rendering reset sheets (normalize etc) with jss? Or do you think those should remain as traditional stylesheets.

[kof]

Ok then.

Side note: Do you see yourself rendering reset sheets (normalize etc) with jss? Or do you think those should remain as traditional stylesheets.

If doing the traditional normalize with global selectors, it doesn't matter if it is in jss or css, however ideally there should be no global normalize at all, but a local instead -> separate topic.

[kof]

What do you think to also being able to set a "start position" for jss by placing in the document beforehand at a chosen place between tags in the head.

Also an interesting option, would give a user all possibilities instead of just 2 beginning/end.

[nathanmarks]

Have you thought of how/where you will implement this? I'm interested in hearing/seeing what you have in mind.

[kof]

I am on #285 right now, do you want to give it a try?

[nathanmarks]

100%, will do tonight after work.

[kof]

So we know now that we want to:

1. be able to define where jss styles start rendering, some sort of entry point.
2. If an entry point is not defined, jss will place styles at the end of the head.
3. Every sheet has an option "index", which defines the relative order of style tags, higher number is closer to the end of style tag, lower number is closer to the beginning.
4. Once we get a new sheet we need to ensure it is inserted in the right order, for e.g. if we get sheet with index 0, then sheet with index 2, then sheet with index 1, they are rendered in the order 0,1,2.

Regarding the entry point, maybe we can even use a comment with specific text like?

<!-- jss -->

[nathanmarks]

@kof There's one aspect I'd like your input on beforehand.

When attaching sheets to the DOM, we need to determine the placement amongst existing sheets. That means that we need to look at the existing sheets that are attached, ordered by priority.

This means that when we either need to insert the sheet into sorted position when we call registry.add() or sort the array every time a sheet is attached. The array is so small that the performance impact is probably non-existent, but do you have a preference?

<!-- jss --> works, only difference is we have to traverse the DOM to find the node instead of finding it via a selector.

[kof]

When attaching sheets to the DOM, we need to determine the placement amongst existing sheets. That means that we need to look at the existing sheets that are attached ordered by priority.

I think rendering backend should get "nextElement" as an option and use .insertBefore when .attach is called, if there is no nextElement, use head.appendChild

This means that when we either need to insert the sheet into sorted position when we call registry.add()

I think we should just place the sheet in the correct order when we call registry.add() by using Array.splice. So yeah we need to iterate the array to find the correct "real" index in the array.

works, only difference is we have to traverse the DOM to find the node instead of finding it via a selector.

I think a comment is more clear and sexy as an entry point than <style data-jss></style>, because we don't actually need the style tag itself, we need the position and can replace it by the first sheet. Iterating head nodes should not be a perf issue, because it is fast and only once.

Correction: we should not replace the comment, because after unmounting everything we don't know the entry point otherwise.

[nathanmarks]

All sounds good to me 👍

[nathanmarks]

Will do this tonight.

[nathanmarks]

2 things came up:

1. Option name? (for priority)
2. Consider link rel="stylesheet" tags when placing before/after? I just realised we didn't discuss that. People can always use the comment to place manually.

[nathanmarks]

Will need a couple of days to make sure this gets done right

The rendering options also need defining. How do you want to store them on the options object? I'm wondering if that options object might get a bit bloated. In some ways it would be easier to keep concerns separated if there was a singleton renderer for each jss instance so it could be initialized and injected by the user.

[kof]

Option name? (for priority)

index?

People can always use the comment to place manually.

Exactly, the idea of manual positioning via comment is better than the previous idea of specifying top/bottom.

In some ways it would be easier to keep concerns separated if there was a singleton renderer for each jss instance so it could be initialized and injected by the user.

The reason for instance per sheet is the style element per sheet renderer needs to save.

[cvle]

Sorry I'm busy with a project's deadline. I just caught up with the conversation and I agree with the current consensus. Looking good so far.

[cvle]

Something to think about: What would happen, when we have multiple JSS instances? What is the expected behavior?

[kof]

1. it will use the same insertion point if any provided
2. it will maintain it's internal order per instance

right @nathanmarks ?

[kof]

done

[kof]

We forgot to document insertion point comment.

[kof]

oh it is in setup, I forgot.