Add support for Web File on Deno

[KnorpelSenf]

Allows File objects to be passed to InputFile (on Deno only). The file will be streamed via file.stream(). The filename will be taken from the supplied file object. Retrying API calls is possible. This change is non-breaking.

@wojpawlik is this possible to port to Node, preferably without raising the minimum Node version?
If no, this will simply be merged, and Node users will not get support for web files.

[wojpawlik]

Node.js support is tricky.

Node 16 and fetch-blob@2 have Blob, but no File. And simple instanceof Blob won't catch all Blob implementations.

I was thinking in terms of allowing File instead of InputFile, and adding name getter and stream method to InputFile to make it File-like.

[KnorpelSenf]

I was thinking in terms of allowing File instead of InputFile, and adding name getter and stream method to InputFile to make it File-like.

InputFile objects are only used with grammY, so there's no need for them to be File-like.

Also, it is a goal to be explicit about the file data that are passed to grammY methods, which is achieved by always relying on the InputFile constructor and nothing else. Permitting File objects contradicts this.

[KnorpelSenf]

If this PR can be merged as-is, it will land in the 1.3 release, which is imminent.

[wojpawlik]

I'd wait with this. Deno has no fileFromPath yet to take advantage of this. And I'll soonish tackle similar problems in https://github.com/telegraf/client and share my thoughts here.

[wojpawlik]

I know what I dislike about this change. This logic should be moved into Client.

[KnorpelSenf]

InputFile is semantically part of the client. The two cannot be separated. It just has to reside in a different file for cross-platform reasons.

In a way, the InputFile encapsulates the runtime-specific part of the client. Given that calling stream() only works on Deno, it needs to be part of InputFile.

That being said, I know this way to divide the code is mediocre at best. Maybe d2n will improve the situation.

[wojpawlik]

d2n should be able to shim Blob, so that it's possible to if (Blob && foo instanceof Blob) anywhere.

[wojpawlik]

Node 16.7.0 added Blob::stream()

[wojpawlik]

For 2.0, InputFile should accept Blob instead of path: string (nodejs/node#37340), and expose just name: string and a .stream() method.

[KnorpelSenf]

That sounds like a good idea, that would simply the abstraction to the client 👍🏻

[wojpawlik]

Further, you could turn InputFile into an interface, and provide class InputFileAsyncIterable instead, perhaps some others. Zero type-switching in your code!

You could still easily detect InputFiles with

function isInputFile(value: any): value is InputFile {
  return value != null && typeof value.stream === 'function'
}

Note that web File already satisfies

interface InputFile {
  readonly name: string
  readonly stream: () => ReadableStream
}

[KnorpelSenf]

Again, the goal is to be explicit about file uploads by constructing an InputFile object. When bot code is read, it should be immediately obvious that a file upload will be performed. Permitting lots of things in the API directly contradicts this because it allows to circumvent the InputFile constructor.

That being said, I'm open to change completely how InputFile and client work together if there is a better solution.

[wojpawlik]

I'd argue that new upload.Buffer(uint8array, name) or new upload.Iterable(streamOrIterable, name) or await Deno.getFile(path) is more explicit than new InputFile(oneOfFourTypes).

I don't think it makes sense to ever merge this PR. Better open a fresh one when working on 2.0.

[wojpawlik]

On 2nd thought, this could be made mergable:

1. Switch to deno2node,
2. Shim Blob,
3. Move Blob detection to Client.

But there doesn't seem to be any demand.

[jimmywarting]

Node 16 and fetch-blob@2 have Blob, but no File. And simple instanceof Blob won't catch all Blob implementations.

fyi, fetch-blob@3 have a File class, it also has a method to get a Blob/File wrapper out of a file path, so it dose not have to have everything in the memory.

Also it has a whatwg compatible .stream() method