Repository navigation
Put a warning against misusing *Sync methods to the API docs. #1684
Description
Taking apart #1665.
While #1674 looks like a good thing, almost eveyone seems to think that it replaces #1665. But it doesn't touch the initial point of #1665 — to discourage using *Sync versions of the methods that could be async.
The reasoning is mostly the same as in #1665.
Two alternatives here:
-
Create a separate page with a warning against improper usage of blocking methods and add a small but instantly visible reference from every
*Syncmethod documentation to that page.
Sample:fs.accessSync(path[, mode])
Warning: this is blocking call, do not use in an asynchronous enviroment unless you know what you are doing. More info on some nice title (link).
Synchronous version of fs.access. This throws if any accessibility checks fail, and does nothing otherwise.
-
Create a separate page with a warning and move all
*Syncmethods documentation to that page, leaving only references (possibly with headers) to it on the corresponding module pages.
Sample:fs.accessSync(path[, mode])
A blocking call, the documentation is listed on some nice title (link).
Notice: I am not a native English speaker and can be a bit wrong with woridings.
Activity
[-]Put a warning against misusing `*Sync` methods in the API docs.[/-][+]Put a warning against misusing `*Sync` methods to the API docs.[/+]benjamingr commentedon May 12, 2015
I'd love it if io took a more active part in the documentation and education of its users about the paradigms it requires like most popular frameworks in other languages do.
This is similar to what MDN is doing but for cases where MDN simply does not apply - we shouldn't document
Function.prototype.applybut it sure would be nice if we gave users a nicer introduction to the platform (vs. the language).I think it would really help if we introduce a guide that explains this sort of stuff to new users - probably on another repo but with a direct link on the website. This can further be translated.
In addition, the official documentation should link there in
Syncmethods. We want to make it clear that these methods, while not deprecated have serious performance overhead for concurrent applications.I think it's also best to accept that a lot of people use node/io.js today for things other than servers and applications. These users should not be discouraged from using node/io.js by the documentation.
CrabDude commentedon May 12, 2015
I think this is where the ecosystem is at evolutionarily. With 1.0, the major runtime concerns are largely addressed, and some manner of canonical education is the next layer beyond API enablement. It's encouraging to know that educational documentation PRs would be considered.
Fishrock123 commentedon May 13, 2015
I'd be in favor of this if it was worded well.
seishun commentedon Oct 16, 2015
Unfortunately, currently Node.js has two distinct groups of functions with "Sync" in them:
fs.readFileSync.Buffer.concat. Time they take is proportional to input size and inversely proportional to CPU speed. Their usage is perfectly fine in most cases. Examples:zlib.deflateSync.I've argued against lumping them together many times before (nodejs/node-v0.x-archive#7030, #5) but didn't get much attention, so this is what we have now. If we're going to warn against using Sync functions in the docs, then we need to clearly explain that they are not all the same.
CrabDude commentedon Oct 20, 2015
@seishun So, IO-bound async (
fs.readFile) and CPU-bound async (zlib.deflate). Your 2 links are good references.When it comes to naming, current APIs prioritize non-blocking (as the default) over faster blocking equivalents in so much that the faster blocking calls require
Sync.[1] See @trevnorris' comments on the OP's referenced #1665 for a slight elaboration on this.My recommendations for any PRs:
Add the following to all
Synccalls:Per @benjamingr suggestion, create a separate guide that concisely covers blocking vs non-blocking and async vs sync while addressing both CPU and IO blocking.
[1] The prioritization order IMO is about identitity. What is node.js' primary value proposition? Is it a _fast_ JavaScript IO-binding runtime? Or a _high-concurrency async non-blocking_ JavaScript IO-binding runtime? Historically, and for the forseeable future, it's definitely the latter, and thus
SyncAPIs should have the aboveWarning: ...in their documentation. Non-blocking single-threaded asynchronicity is also The JavaScript Way™ because it lacks all multi-threading supportAs an aside, I predict all async counterparts to
SyncAPIs will eventually be renamed withAsyncappended in conjunction with the transition towardasync/await&Promiseand to better distinguish between functions likeurl.parseandzlib.deflate. Ironically (and intentionally on my part), in this example, the difference is somewhat arbitrary because both are CPU-bound.evanlucas commentedon Jan 20, 2016
/cc @nodejs/documentation This is something I feel would be very beneficial for newcomers
benjamingr commentedon Jan 20, 2016
Still +1 on this.
Fishrock123 commentedon Jun 29, 2016
Now that we have special ymal metadata maybe we could make this a special field that appears is a specific way with an expandable tooltip? //cc @nodejs/documentation
Trott commentedon Jul 9, 2017
@ChALkeR This should remain open?
wuweiweiwu commentedon Feb 27, 2018
Can I pick this up? I think it'll be a good first issue :)
benjamingr commentedon Feb 27, 2018
Actually, this is mostly just confusing new users, I'm going to go ahead and close this (anyone feel free to reopen) given the outcome of #17503
@wuweiweiwu if you're looking for new issues or things to tackle - I think our ESM modules docs could use some love and testing :)
wuweiweiwu commentedon Feb 27, 2018
@benjamingr Sounds good. Where could I find more information on ESM modules / tests needed to be done? Thank you!
benjamingr commentedon Feb 27, 2018
@wuweiweiwu in general, https://github.com/nodejs/node/blob/master/doc/api/esm.md is lacking compared to https://github.com/nodejs/node/blob/master/doc/api/modules.md - I think we should gradually add relevant information and usage examples to the ESM docs
wuweiweiwu commentedon Feb 28, 2018
@benjamingr Ok! Ill open a WIP PR soon after I read more and get more familiar