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

Feature - add option to further pipe stream #18

Merged
merged 3 commits into from
May 28, 2020

Conversation

dpnolte
Copy link
Contributor

@dpnolte dpnolte commented May 7, 2020

Hi,

This PR aims to expose the serveFragment stream to the library user. That is, an option to provide any extra pipes on the stream is provided. With this option, users can further transform the fragment output as desired. I created this PR because I needed to add rendered styles from emotion to the served fragment.

Changes:

  • options I've added an options argument to serveFragments. I was assuming that more options could follow
  • options.pipeStream After removing the react root transform stream, it will check if option.pipeStream is provided and executes it.

I'd be happy to implement any desired changes.

src/server.tsx Outdated
const removeReactRootStream = new RemoveReactRoot();
stream.pipe(
removeReactRootStream,
{ end: false }
);
removeReactRootStream.pipe(

let lastStream: NodeJS.ReadableStream = removeReactRootStream;
Copy link
Owner

@dunglas dunglas May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let lastStream: NodeJS.ReadableStream = removeReactRootStream;
const lastStream: NodeJS.ReadableStream = options.pipeStream ? options.pipeStream(removeReactRootStream) : removeReactRootStream;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks that is cleaner indeed. I've added it in commit 047a3db:

const lastStream: NodeJS.ReadableStream =  options.pipeStream ? options.pipeStream(removeReactRootStream) : removeReactRootStream;

@dunglas dunglas merged commit dbe0562 into dunglas:master May 28, 2020
@dunglas
Copy link
Owner

dunglas commented May 28, 2020

Thanks @dpnolte

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants