Simplify API for streaming and fix unmatchable bug #16
Conversation
7rulnik
changed the title
7rulnik
force-pushed
the
feature/simplify-streaming-api
branch
from
bf18808
to
d81d884
Compare
| @@ -73,4 +74,11 @@ export const createCriticalStyleStream = (def: StyleDefinition) => { | |||
| cb(undefined, line.tail); | |||
| } | |||
| }); | |||
|
|
|||
| reactStream.on('error', err => { | |||
There was a problem hiding this comment.
this might be an important moment.
| @@ -14,8 +14,8 @@ input { display: none; } | |||
| `; | |||
|
|
|||
| exports[`React css stream React.renderToStream 1`] = ` | |||
| "input { color: rightInput; } | |||
| <style type=\\"text/css\\" data-used-styles=\\"file1,file2\\">.a, | |||
| "<style type=\\"text/css\\" data-used-styles=\\"true\\">input { color: rightInput; } | |||
| @@ -106,8 +106,9 @@ export const extractAllUnmatchable = kashe((def: StyleDefinition): StyleChunk[] | |||
| )); | |||
|
|
|||
| export const extractAllUnmatchableAsString = kashe((def: StyleDefinition) => ( | |||
| extractAllUnmatchable(def) | |||
| .reduce((acc, {css}) => acc + css, '') | |||
| wrapInStyle( | |||
PS: It would be very good to have a bug fix a separate PR to publish it without any delay. |
|
Hey! I've updated underlaying tools and updated tslint/prettier caused some conflicts :( |
|
You stole my contribution :D |
|
@theKashey I thought @7rulnik is right about simplifying of API without using |
|
I thought there is not much usage of this library for now and breaking API change is not big deal. |
|
We discussed a bit this idea with @theKashey. He wants to save flexibility of api (for example, you can pipe into used-styles directly). I agree with him so I will introduce Somethin like that: combine(reactStream, usedStylesStream) |
|
@theKashey looks like it could be revived :D Also maybe we need some changes to adopt it for React 18 |
|
👋 hey, there is a good time to break API in some way to support #40. That would require internal API to become a little bit more abstract so one can build more specific code on top of. So can you clarify what exactly you are looking for and let's do it.
I've used used-styles for React 18 in "old good rendering mode" with no issues (and there could be none), but I am really not sure how to support the "new rendering mode". |
We can simplify API for users by hiding pipe inside of createStyleStream/createCriticalStyleStream. So users will just wrap React stream with our function. So the whole render process becomes simpler because we don't need multistream dep and also it's easier to catch errors because now we forward error from React stream.
I don't understand your idea with block rendering (I don't understand what headerStream stands for) so it's kinda hard to update usage in this block.
Also, I can make it with backward capability but not sure that it worth it.
And there is a problem with
extractAllUnmatchableAsString. It didn't wrap css into style tag. I fixed it in this PR, but I can create another one for it.