_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:
*Sync method documentation to that page.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._
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.apply but 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 Sync methods. 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:
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 (https://github.com/nodejs/node-v0.x-archive/issues/7030, https://github.com/nodejs/node/issues/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.
@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 https://github.com/nodejs/node/issues/1665 for a slight elaboration on this.
My recommendations for any PRs:
Sync calls: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 Sync APIs should have the above Warning: ... 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 Sync APIs will eventually be renamed with Async appended in conjunction with the transition toward async/await & Promise and to better distinguish between functions like url.parse and zlib.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
@ChALkeR This should remain open?
Can 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 https://github.com/nodejs/node/pull/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
Most helpful comment
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