isPromiseFs fails on Deno

[mdekstrand]

isPromiseFs fails on Deno

[mdekstrand]

I have confirmed that denoland/deno#22030 fixes this on the Deno side, so the next Deno release (due tomorrow) should work.

[jcubic]

Do you want to contribute and create a fix?

As for the catch in Deno, maybe this should reported as a bug for Deno. Errors should not escape catch unless the error is retrown.

[mdekstrand]

I would definitely be open to contributing a fix. I've also reported a bug against Deno (denoland/deno#21795), and will see what they say there first. It definitely seems like a bug, even if it's really weird to call readFile() without an argument.

[mdekstrand]

@jcubic Thinking about this, is there a reason it needs to check that fs.promises is actually promise-like? I see why it needs to check if you just pass fs, but could that check be dropped without causing other problems?

Alternatively, would it work to provide a way to say “this one is really a promise-based api, trust me”?

[jcubic]

The problem is that the library expects to detect if it's normal FS or promisified FS. If it's normal FS it promisify all the functions from FS. So yes it needs to check if the result is fs.promise or not. But maybe it can check different method that don't throw an error.

I'm wondering if fs.stats will work on a root directory. It needs to work for both browser and Node (and in your case Deno). It needs to be a method that is included in lightningFS.

[jcubic]

You can also report this bug to Deno. Exceptions should not bypass try..catch.

[mdekstrand]

It's being discussed over at Deno, and this comment gets to what's happening here: denoland/deno#21795 (comment).

I think there's a decent case to be made either way — it looks like an emergent property of JavaScript's defaulting behavior, the fact that promisify is not aware of method signatures, and calling the readFile out-of-spec. It looks like explicitly passing undefined to readFile in isPromiseLike would fix or work around it from the isomorphic-git side.

[mdekstrand]

Deno folks are also working on a patch: denoland/deno#22030

[jcubic]

You can contribute with a fix. Maybe using an empty string will look better than passing undefined. I'm not sure how Deno will work for that case.