FileSystemAccess.prototype.readText has an async interface, but is synchronous

[moll]

Hey,

I was going through the source code and stumbled upon something pretty weird.

new FileSystemAccess().readText(name, onSuccess, onError)

That's an async interface, but according to https://github.com/NativeScript/NativeScript/blob/5e5d75c09d8a3a49bbf6de5bfa53bc1736e99de4/apps/tests/file-system-access-tests/file-system-access-tests.ts and its use in builder.ts, it works synchronously. That's zalgo right there. A sync interface is handy, but the weird interface is, well, weird. It should return the content of the file on success and throw on error.

Cheers.

[enchev]

Indeed file access is synchronous for now however once we have support for background workers/threads we will improve this.

[moll]

Till then, the API should be renamed to, e.g., Node style's readFileSync and skip the callback faux pas. ;-)

[atanasovg]

I think we should emphasize on the usage of the file-system module rather than the file-system-access one. The first one should be the promoted/suggested way for accessing the file system, while the second one should not be publicly accessed. Either way, we need to extend the API with the sync suffix which semantically describes the operation.

[moll]

As per #554 (comment) this might just not yet be finished.

[atanasovg]

The onError parameter is optional and it actually provides another means to try-catch your code. Is try-catch the preferred (or kosher) way to do error handling?

[moll]

Hey. You're right, it's not really the best solution for a synchronous API. It reinvents what try-catch is there for. It's also not clear to read as why should a synchronous API take a callback. I haven't checked how the async versions now behave (perhaps there still aren't any).

Anywho, synchronous APIs in JavaScript should stick to returning a value or throwing an exception. Theoretically that could be implemented in union types to forgo exceptions, but I wouldn't. Most other [sync] APIs are exception based here. https://www.joyent.com/developers/node/design/errors is a decent enough reference to start thinking about error design.

Once you get to async interfaces, I wouldn't invent the wheel either. Promises or Node.js style continuation-passing with the same error API and you're good to go. ;-) Feel free to give me a call someday if you want to chat about APIs.

[atanasovg]

@moll thanks for the heads-up :) I tend to agree with you and the suggested way for error handling. Truth is the file-system module was one of the first modules in the early days of {N} and we weren't sure how exactly to write asynchronous APIs - Node vs. Cordova vs. Promises. We started the Cordova way and obviously we've made some mixtures of both synchronous and asynchronous approaches.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.