@uppy/transloadit: implement Server-sent event API

[aduh95]

Could replace Socket.io to get Assembly status update without HTTP polling. (See https://developer.mozilla.org/en-US/docs/Web/API/EventSource, and https://caniuse.com/eventsource)

[mifi]

not sure why I was requested to review this, as it doesn't seem to be finished, but it seems like a good idea if websockets are hard to get working.
should we remove websockets code or do we need to support both at the same time?

[arturi]

try/catch JSON.parse in case of an error?

[aduh95]

Not sure it'd be useful, that would only happen if there's a bug on Transloadit side, at which point throwing an error seems like the correct behavior.

[arturi]

Would it make sense to add a setTimeout to do fetchStatus if no events were emitted from SSE for X seconds?

[Murderlon]

Shouldn't it be this?

Suggested change

	this.emit('result', stepName, result)
	this.emit('transloadit:result', stepName, result)

[aduh95]

uppy/packages/@uppy/transloadit/src/Assembly.js

Lines 103 to 109 in 5ec7187

	socket.on('assembly_result_finished', (stepName, result) => {
	this.emit('result', stepName, result)
	if (!this.status.results[stepName]) {
	this.status.results[stepName] = []
	}
	this.status.results[stepName].push(result)
	})

[Murderlon]

Suggested change

	this.emit('executing')
	this.emit('transloadit:assembly-executing')

[aduh95]

This is copied from

uppy/packages/@uppy/transloadit/src/Assembly.js

Lines 94 to 96 in 5ec7187

	socket.on('assembly_uploading_finished', () => {
	this.emit('executing')
	})

Changing the event would be a breaking change, right?

[arturi]

Gotta be careful here indeed, as some of the events are Assembly internal events, not Uppy Core event emitter, it's confusing.

[Murderlon]

internal only?

Suggested change

	this.emit('metadata')
	// used only internally to do X
	this.emit('metadata')

[aduh95]

No idea, I've copied it from

uppy/packages/@uppy/transloadit/src/Assembly.js

Lines 98 to 101 in 5ec7187

	socket.on('assembly_upload_meta_data_extracted', () => {
	this.emit('metadata')
	this.#fetchStatus({ diff: false })
	})

[mifi]

LGTM except I'm a bit confused why some events are handled in this.#sse.addEventListener('message'), e.g. assembly_uploading_finished and assembly_upload_meta_data_extracted, while others in their own eventListeners, e.g. this.#sse.addEventListener('assembly_upload_finished')? is there a good reason why everything isn't just in the message event? should it be documented why?

[aduh95]

The upstream is sending events, sometimes as 'message' (which is the default), sometimes as a custom event. But event must send data with them, while Transloadit.com could work around this by sending an empty string or a dummy one, sending it as message sounds like a cleaner way of doing things.

should it be documented why?

Yes, on Transloadit side. We're waiting to have a successful implementation with Uppy before officialising the API (after that, it will be quite hard to make breaking changes if we have to)