Repository navigation
Put a warning against misusing *Sync methods to the API docs. #1684
Description
Activity
- changed the title
[-]Put a warning against misusing `*Sync` methods in the API docs.[/-][+]Put a warning against misusing `*Sync` methods to the API docs.[/+]on 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.
it sure would be nice if we gave users a nicer introduction to the platform (vs. the language).
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.
I'd be in favor of this if it was worded well.
Unfortunately, currently Node.js has two distinct groups of functions with "Sync" in them:
- Functions that block on I/O. They literally waste time and can take unpredictable amounts of time. It's almost always a bad idea to use them outside of the first tick. Examples:
fs.readFileSync. - Functions that happen to have an alternative that runs in a thread pool. Otherwise they work exactly the same as, for example,
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.
- Functions that block on I/O. They literally waste time and can take unpredictable amounts of time. It's almost always a bad idea to use them outside of the first tick. Examples:
@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:fs.accessSync(path[, mode])
Warning: This is a blocking call. See the blocking vs non-blocking guide. (link)
-
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 support
As 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.-
/cc @nodejs/documentation This is something I feel would be very beneficial for newcomers
Still +1 on this.
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
Reacted by Anna Henningsen and Evan LucasCan I pick this up? I think it'll be a good first issue :)
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 :)
@benjamingr Sounds good. Where could I find more information on ESM modules / tests needed to be done? Thank you!
@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
@benjamingr Ok! Ill open a WIP PR soon after I read more and get more familiar
Reacted by Benjamin Gruenbaum
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
*Syncversions 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:
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:
Notice: I am not a native English speaker and can be a bit wrong with woridings.