doc: note caveats in process message serialization#12963
Uh oh!
There was an error while loading. Please reload this page.
Conversation
joyeecheungforce-pushed the process-send-caveat branch from 8a3b2e1 to 6e47903Comparejoyeecheung added child_process 
doc/api/child_process.md Outdated
There was a problem hiding this comment.
Perhaps simplify the link and adjust some of the wording here... for example:
*Note*: since the message is transmitted as JSON, not all values may be transferred as-is. See the documentation for [`JSON.stringify()`][] for more details.
Where the JSON.stringify() link goes to MDN instead (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#Description), which IMHO has a more readable description of when/how values are converted.
There was a problem hiding this comment.
The MDN documentation doesn't mention the Nan/Infinity -> null thing which I think is one of the surprising behaviors..the link is not about the serialization process, but rather, the caveats, which are described in the notes of the ECMA spec (although I cannot find a direct anchor to those notes, so I use the link to the method instead).
There was a problem hiding this comment.
Also the MDN documentation doesn't mention the circular reference error...:/ hmm
There was a problem hiding this comment.
Nit: The comma after sent should be a period. See notes at... can be its own sentence.
doc/api/child_process.md Outdated
There was a problem hiding this comment.
I would just use the exact same text as suggested above for this part.
doc/api/process.md Outdated
doc/api/process.md Outdated
doc/api/process.md Outdated
There was a problem hiding this comment.
Identical nit as above about comma and see notes.
cjihrig left a comment
cjihrig left a comment There was a problem hiding this comment.
I think the existing messaging (Note: this function uses [JSON.stringify()][] internally to serialize the message.) is good. The message should probably be moved to a better location in the send() docs though.
I don't think we should explain too much about how stringify() works, when we are linking to it already.
doc/api/child_process.md Outdated
There was a problem hiding this comment.
JSON.stringify() is already in this list of links in both of these files.
doc/api/process.md Outdated
There was a problem hiding this comment.
please keep the refs sorted
BridgeAR commented Aug 26, 2017
Ping @joyeecheung this needs a rebase and the comments still need to be addressed |
Trott commented Aug 27, 2017
@joyeecheung I'm happy to push a commit for fixes for my nits if you'd like. Just let me know. |
BridgeAR commented Sep 19, 2017
Ping @Trott do you want to follow up on this? I would otherwise close this. |
The message sent using process.send() goes through JSON serialization and parsing, which could lead to surprising behaviors. This commit elaborate a bit more on this and add a link to the notes about these caveats in the ECMAScript specification.
Trottforce-pushed the process-send-caveat branch from 6e47903 to 9d99868CompareTrott commented Sep 20, 2017
I rebased, resolved conflicts, fixed all nits, and pushed back up to Joyee's branch. I think this is ready to land. @cjihrig Can you review and clear your request for changes if this addresses your concerns? I'm not sure if the new wording I settled on is OK by you or not. @joyeecheung Can you review and confirm that what I've done here meets your intentions? @nodejs/documentation PTAL |
BridgeAR commented Sep 22, 2017
Landed in ba5a668 (I felt free to land this due to no response from @joyeecheung since a month) |
The message sent using process.send() goes through JSON serialization and parsing, which could lead to surprising behaviors. This commit elaborate a bit more on this and add a link to the notes about these caveats in the ECMAScript specification. PR-URL: #12963 Refs: #12497 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
The message sent using process.send() goes through JSON serialization and parsing, which could lead to surprising behaviors. This commit elaborate a bit more on this and add a link to the notes about these caveats in the ECMAScript specification. PR-URL: nodejs/node#12963 Refs: nodejs/node#12497 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
The message sent using process.send() goes through JSON serialization and parsing, which could lead to surprising behaviors. This commit elaborate a bit more on this and add a link to the notes about these caveats in the ECMAScript specification. PR-URL: #12963 Refs: #12497 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
The message sent using process.send() goes through JSON serialization and parsing, which could lead to surprising behaviors. This commit elaborate a bit more on this and add a link to the notes about these caveats in the ECMAScript specification. PR-URL: #12963 Refs: #12497 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Vse Mozhet Byt <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
10 participants
The message sent using process.send() goes through JSON
serialization and parsing, which could lead to surprising behaviors.
This commit elaborate a bit more on this and add a link to
the notes about these caveats in the ECMAScript specification.
Refs: #12497
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
doc, process, child_process