Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

worker.send stripping Map and replacing with {} #10965

Closed
davebriand opened this issue Jan 23, 2017 · 12 comments
Closed

worker.send stripping Map and replacing with {} #10965

davebriand opened this issue Jan 23, 2017 · 12 comments
Assignees
Labels
child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers.

Comments

@davebriand
Copy link

  • Version:v6.9.4
  • Platform:Darwin Daves-MacBook-Air-2.local 14.3.0 Darwin Kernel Version 14.3.0: Mon Mar 23 11:59:05 PDT 2015; root:xnu-2782.20.48~5/RELEASE_X86_64 x86_64
  • Subsystem:
let worker = cluster.fork();
let myMap = new Map();
myMap.set('s', {foo: 'bar'});
worker.send({
  myMap: myMap
});

When the worker gets the message myMap becomes {}
When sending an array or object it works as expected

@davebriand davebriand changed the title worker.send striping Map and replacing with {} worker.send stripping Map and replacing with {} Jan 23, 2017
@cjihrig
Copy link
Contributor

cjihrig commented Jan 23, 2017

This is because send() calls JSON.stringify() on the Map. As a workaround, you can define a toJSON() method for your Map.

@mscdex mscdex added child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers. labels Jan 23, 2017
@TimothyGu
Copy link
Member

Example solution that should work:

class SerializableMap extends Map {
  toJSON() {
    const obj = Object.create(null);
    for (const [key, value] of this) {
      obj[key] = value;
    }
    return obj;
  }
}

let worker = cluster.fork();
let myMap = new SerializableMap();
myMap.set('s', {foo: 'bar'});
worker.send({
  myMap
});

Other than that @cjihrig covered everything already, so closing.

@addaleax addaleax self-assigned this Mar 20, 2017
@addaleax
Copy link
Member

@TimothyGu are you okay with me reopening this? I’d like to see how far we can get with the ValueSerializer stuff once #11048 lands. (I’ll assign this issue to myself.)

@addaleax addaleax reopened this Mar 20, 2017
@TimothyGu
Copy link
Member

@addaleax no problem at all. Just trying to cleaning up some old and inactive issues.

@Trott
Copy link
Member

Trott commented Jul 30, 2017

@addaleax Is this still something you intend to look at? Or should this be closed?

@addaleax
Copy link
Member

Yes, but it’s not something we can realistically do until V8 moves its API out of experimental status, I think (because this is about a non-experimental API of ours).

@BridgeAR
Copy link
Member

BridgeAR commented Sep 23, 2017

Is this something we still want to pursue? In that case I guess it makes sense to add the help wanted label (I am not sure if the API is still experimental though but I hope not)?

@apapirovski
Copy link
Member

@addaleax @TimothyGu Could you give a quick status update on this? I'm not familiar enough with V8 to know where this is at. Thank you!

@bnoordhuis
Copy link
Member

Still experimental. There's also the issue that JSON is the documented serialization protocol so there might be applications or libraries out there relying on quirks of JSON.parse() and JSON.stringify().

@Trott
Copy link
Member

Trott commented Nov 11, 2018

Might someone supply another status update on this one for those of us who don't follow V8 and wouldn't know where to look?

@addaleax
Copy link
Member

@nodejs/v8 ^^^

(tl;dr: When does ValueSerializer/ValueDeserializer become no longer experimental?)

@fhinkel
Copy link
Member

fhinkel commented Oct 29, 2019

ping @addaleax

addaleax added a commit to addaleax/node that referenced this issue Nov 1, 2019
Add an `serialization` option that allows child process IPC to
use the (typically more powerful) V8 serialization API.

Fixes: nodejs#10965
MylesBorins pushed a commit that referenced this issue Nov 17, 2019
Add an `serialization` option that allows child process IPC to
use the (typically more powerful) V8 serialization API.

Fixes: #10965

PR-URL: #30162
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit to targos/node that referenced this issue Jan 13, 2020
Add an `serialization` option that allows child process IPC to
use the (typically more powerful) V8 serialization API.

Fixes: nodejs#10965

PR-URL: nodejs#30162
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
targos pushed a commit that referenced this issue Jan 13, 2020
Add an `serialization` option that allows child process IPC to
use the (typically more powerful) V8 serialization API.

Fixes: #10965

PR-URL: #30162
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
BethGriggs pushed a commit that referenced this issue Feb 6, 2020
Add an `serialization` option that allows child process IPC to
use the (typically more powerful) V8 serialization API.

Fixes: #10965

PR-URL: #30162
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. question Issues that look for answers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants