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

Refactor Worker and NodeMainInstance class to reuse code #29925

Open
joyeecheung opened this issue Oct 11, 2019 · 2 comments
Open

Refactor Worker and NodeMainInstance class to reuse code #29925

joyeecheung opened this issue Oct 11, 2019 · 2 comments
Labels
process Issues and PRs related to the process subsystem.

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Oct 11, 2019

Opening an issue to discuss about how to refactor the Worker and NodeMainInstance class to reuse code.

My current plan is to create a base class NodeInstance and try to strip out common code in WorkerData/Worker as well as NodeMainInstance in there, and then make Worker and NodeMainInstance inherit from NodeInstance.

This is useful in adding support for startup snapshots in workers and ContextifyContext - otherwise we need to repeat e.g. snapshot availability detection code in multiple places which can be tricky to maintain.

Refs: #29842

@joyeecheung
Copy link
Member Author

cc @addaleax

@joyeecheung joyeecheung added the process Issues and PRs related to the process subsystem. label Oct 11, 2019
@joyeecheung
Copy link
Member Author

I realized that we cannot actually deserialize the default context from the snapshot if the user uses the sandbox argument of vm.createContext, since the context would be different from the one we have for our own Node.js instances. We could still do this for contexts created without the sandbox argument and the users can work around the limitation by setting up the sandbox though property setters after the call to vm.createContext, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
process Issues and PRs related to the process subsystem.
Projects
None yet
Development

No branches or pull requests

1 participant