Repository navigation
FileSystemAccess.prototype.readText has an async interface, but is synchronous #537
Description
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.
Activity
enchev commentedon Aug 3, 2015
Indeed file access is synchronous for now however once we have support for background workers/threads we will improve this.
moll commentedon Aug 3, 2015
Till then, the API should be renamed to, e.g., Node style's
readFileSyncand skip the callback faux pas. ;-)atanasovg commentedon Aug 3, 2015
I think we should emphasize on the usage of the
file-systemmodule rather than thefile-system-accessone. 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 thesyncsuffix which semantically describes the operation.moll commentedon Aug 11, 2015
As per #554 (comment) this might just not yet be finished.
atanasovg commentedon Aug 11, 2015
The onError parameter is optional and it actually provides another means to
try-catchyour code. Istry-catchthe preferred (or kosher) way to do error handling?moll commentedon Aug 11, 2015
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 commentedon Aug 12, 2015
@moll thanks for the heads-up :) I tend to agree with you and the suggested way for error handling. Truth is the
file-systemmodule 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 commentedon Aug 31, 2019
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.