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

SSR via Cloudflare Workers & renderToWebStream (3.2 ) #4243

Closed
Cherry opened this issue Aug 3, 2021 · 11 comments
Closed

SSR via Cloudflare Workers & renderToWebStream (3.2 ) #4243

Cherry opened this issue Aug 3, 2021 · 11 comments

Comments

@Cherry
Copy link
Contributor

Cherry commented Aug 3, 2021

Version

3.2.0-beta.7

Reproduction link

https://github.com/Cherry/workers-vue-ssr-example

The src/index.mjs file contains a really simple Hello World Vue App, attempting to be rendered server-side. It has 3 different solutions in the code, in this order:

  1. Native
  2. via TransformStream
  3. Buffering stream in memory

Each is commented out, with the only one actually working being 3, buffering in memory.

Steps to reproduce

See the attached reproduction repo for more information.

What is expected?

When using SSR and renderToWebStream, Options 1 and/or 2 should work without issue. The docs in @vue/server-renderer seem to suggest this is possible, but all attempts have resulted in errors.

What is actually happening?

Options 1 and 2 throw errors, either relating to ReadableStream not being a constructor, or there being no response. More information can be found in the README of the attached reproduction repo.


If I can provide any more information, please let me know and I'll be happy to.

@lidlanca
Copy link
Contributor

lidlanca commented Aug 3, 2021

cloudflare web worker does not implement ReadableStream instead they provide TransformStream
https://developers.cloudflare.com/workers/runtime-apis/streams/transformstream

you can use a ponyfills of ReadableStream, and then
read the stream, and "pipe" the values yourself to the writer of TransformStream

 let { readable, writable } = new TransformStream()

 let reader = renderToWebStream(...)
 pipeReaderToWriter(reader, writable) // async, read stream from 'reader' and write to `writable`, close writer when done.
 return new Response(readable)

@Cherry
Copy link
Contributor Author

Cherry commented Aug 3, 2021

I tried something like that as per example 2, and https://github.com/Cherry/workers-vue-ssr-example/blob/c195fea2404e4cfd3b02c763ab483877ae1f185e/src/index.mjs#L23-L29. Is this not the correct way to do this?

@lidlanca
Copy link
Contributor

lidlanca commented Aug 3, 2021

The polyfill and the cloudflare native implementation may not play nice.

that is why I suggested pipeReaderToWriter()

async pipeReaderToWriter(r,w) function(){
   for(;;) {
      let {value, done} = await r.read()
      await w.write(value)
      if(done) {
        break
      }
   } 
   w.close()
}

@Cherry
Copy link
Contributor Author

Cherry commented Aug 3, 2021

Thanks for the additional insight. Unfortunately that appears to result in the same error as described in example 2, with The script will never generate a response.. Specifically with:

async function pipeReaderToWriter(reader, writer){
	for(;;){
		const {value, done} = await reader.read();
		console.log('write value', value);
		await writer.write(value);
		if(done){
			break;
		}
	}
	writer.close();
}

And

const {readable, writable} = new TransformStream();
const render = renderToWebStream(app, {}, polyReadableStream);
pipeReaderToWriter(render.getReader(), writable);
return new Response(readable);

The docs at https://github.com/vuejs/vue-next/tree/master/packages/server-renderer#rendertowebstream seem to imply this should be a lot simpler. I'm wondering if there's something really obvious I'm missing.

@lidlanca
Copy link
Contributor

lidlanca commented Aug 3, 2021

pipeReaderToWriter(render.getReader(), writable.getWriter());

@Cherry
Copy link
Contributor Author

Cherry commented Aug 3, 2021

🤦 Of course, thank you.

I had to tweak the encoding to ensure I was only writing UInt8Arrays as per the error This TransformStream is being used as a byte stream, but received a string on its writable side. If you wish to write a string, you'll probably want to explicitly UTF-8-encode it with TextEncoder., but this ended up looking like:

async function pipeReaderToWriter(reader, writer){
	const encoder = new TextEncoder();
	for(;;){
		const {value, done} = await reader.read();
		const encoded = encoder.encode(value);
		await writer.write(encoded);
		if(done){
			break;
		}
	}
	writer.close();
}
const {readable, writable} = new TransformStream();
const render = renderToWebStream(app, {}, polyReadableStream);
pipeReaderToWriter(render.getReader(), writable.getWriter());
return new Response(readable);

Would a PR to update the docs at https://github.com/vuejs/vue-next/tree/master/packages/server-renderer?rgh-link-date=2021-08-03T23%3A30%3A34Z#rendertowebstream be a good idea?

@lidlanca
Copy link
Contributor

lidlanca commented Aug 4, 2021

This is very specific to cloudflare web workers and their non smooth integration with the polyfill

in my opinion if this is a common use case for cloudflare web workers, it should be documented somewhere
to save people some time, may be in the readme, or maybe in the v3 docs / guide.
or just this issue.

@Cherry
Copy link
Contributor Author

Cherry commented Aug 4, 2021

Agreed it should be documented somewhere, or the line at https://github.com/vuejs/vue-next/tree/master/packages/server-renderer?rgh-link-date=2021-08-03T23%3A43%3A16Z#rendertowebstream that says // e.g. inside a Cloudflare Worker simply removed, or linked somewhere else for more information, since it's not the simple as the docs currently imply.

@yyx990803
Copy link
Member

I adjusted the streaming API in 86d78d1 and added pipeToWebWritable which should be more flexible in such scenarios:

const { readable, writable } = new TransformStream()
pipeToWebWritable(app, {}, writable)

return new Response(readable)

Will release a beta today so you can test it out.

@Cherry
Copy link
Contributor Author

Cherry commented Aug 6, 2021

This is fantastic, thanks!

@Cherry
Copy link
Contributor Author

Cherry commented Aug 7, 2021

Just tested this @yyx990803. Unfortunately it throws the error:
Error: Failed to get the 'ready' property on 'Writer': the property is not implemented.

It seems this is another place where Cloudflare Workers divert from the spec.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants