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 in Cloudflare Workers (TransformStream and ReadableStream) #4287

Closed
Cherry opened this issue Aug 9, 2021 · 6 comments
Closed

SSR in Cloudflare Workers (TransformStream and ReadableStream) #4287

Cherry opened this issue Aug 9, 2021 · 6 comments

Comments

@Cherry
Copy link
Contributor

Cherry commented Aug 9, 2021

This is a follow-up to #4243, which unfortunately has not been resolved yet and I thought I'd surface a new issue to bring attention to this since it was mentioned in today's blog post.

In that issue, a new API pipeToWebWritable was added to use Cloudflare's TransformStream implementation, but unfortunately doesn't seem to be compatible with their implementation. Unfortunately that same commit seemed to remove the older method of passing a ponyfilled ReadableStream into renderToWebStream too, so there's no real way to do this right now without another workaround.

As a side note, the docs still suggest that passing a third option to renderToWebStream is still possible, even though this was removed.

Version

3.2.1

Reproduction link

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

Steps to reproduce

What is expected?

This API should work as expected and not throw an error. I'd propose that the older beta-7 functionality be restored in renderToWebStream (with passing in a ponyfilled ReadableStream) so this can at least be hacked around for now, or the functionality in pipeToWebWritable tested/extended to support Cloudflare Workers more directly.

What is actually happening?

API throws Error: Failed to get the 'ready' property on 'Writer': the property is not implemented..


@Cherry Cherry changed the title SSR in Cloudflare Workers (TransportStream and ReadableStream) SSR in Cloudflare Workers (TransformStream and ReadableStream) Aug 9, 2021
@lidlanca
Copy link
Contributor

lidlanca commented Aug 9, 2021

workaround cloudflare limitation is still possible

  function cloudflareWriteableCompatibility(writeable){
    return {
      getWriter(){
        var w = writeable.getWriter()
        w.ready = Promise.resolve()
        return w
      }
    }
  }
 let { readable, writable } = new TransformStream()

// wrap cloudflare writable stream with a compatibility layer to support .ready promise
var writableComp = cloudflareWriteableCompatibility(writeable)

pipeToWebWritable(input,ctx, writableComp)

return new Response(readable)

@Cherry
Copy link
Contributor Author

Cherry commented Aug 10, 2021

Unfortunately not. I tried something like that and every time it threw Cannot assign to read only property 'ready' of object '#<Writer>'.

The only way I've come close is with a hacky proxy, and something like this:

const cloudflareWriteableCompatibility = function(writable){
	// serious hack. We can't just assign `ready` to `writable` because it's read-only.
	// so let's use a proxy, return something fake before ready, and then on ready
	// return the real writer
	let ready = false;
	const writerProxy = new Proxy(writable, {
		get: function(target, property){
			if(property === 'ready'){
				ready = true;
				return Promise.resolve();
			}
			if(typeof(target[property]) === 'function'){
				if(!ready){
					// not ready yet. Return ourselves, which will contain a `ready` func
					return function(){
						return writerProxy;
					};
				}
				// ready now, do the real thing
				return function(...args){
					return target[property].apply(target, args);
				};
			}
			return Reflect.get(...arguments);
		},
	});
	return writerProxy;
};
const app = createSSRApp(appVue);
const {readable, writable} = new TransformStream();
const writableComp = cloudflareWriteableCompatibility(writable);
pipeToWebWritable(app, {}, writableComp);
return new Response(readable);

But this yields a The script will never generate a response. error.

@lidlanca
Copy link
Contributor

lidlanca commented Aug 10, 2021

I dont see a reason for the proxy not to work as long as you can override the ready.

Assuming your logic is correct.

Try debugging and see if the writer close method is actually called and properly delegated to the actual writer

I didnt really get the extra ready logic

@yyx990803
Copy link
Member

Hmm, I really think CloudFlare workers should at least be implemented in a spec compliant fashion.

Theoretically this can be worked around by checking the presence of ready with 'ready' in writer in pipeToWebWritable?

@yyx990803
Copy link
Member

This should be fixed, but the special case should be removed in the long run when CF workers update their stream implementations to be spec-compliant (they seem to be willing to).

Also as a workaround it is quite easy for you to implement a CF-worker-compatible pipeToWebWritable yourself using renderToSimpleStream: https://github.com/vuejs/vue-next/blob/master/packages/server-renderer/src/renderToStream.ts#L173-L204

@Cherry
Copy link
Contributor Author

Cherry commented Aug 10, 2021

Perfect, thank you so much. I can confirm that implementing something akin to your changes with a manual renderToSimpleStream now works as expected!

@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.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants