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: take advantage of structured clone algorithm in the remote module #20427

Merged
merged 1 commit into from Oct 10, 2019

Conversation

@miniak
Copy link
Contributor

commented Oct 4, 2019

Description of Change

Take advantage of structured clone algorithm based IPC introduced by #20214:

  • Date, ArrayBuffer and ArrayBufferView can now be serialized as values.
  • Keeping special handling to preserve Buffer, otherwise it will be converted to UInt8Array.
  • Use native serialization for Boolean, Number, String and RegExp.
  • Error is now serialized natively. Preserve custom properties in the remote module.

Checklist

Release Notes

Notes: The remote module now properly serializes Boolean, Number, String and RegExp instances.

@miniak miniak referenced this pull request Oct 4, 2019
6 of 6 tasks complete
@miniak miniak added the wip label Oct 4, 2019
@miniak miniak self-assigned this Oct 4, 2019
@electron-cation electron-cation bot removed the new-pr 🌱 label Oct 5, 2019
@miniak miniak force-pushed the miniak/simplify-remote branch 3 times, most recently from 22a6894 to d4d7b2b Oct 7, 2019
@miniak miniak changed the base branch from cloneable-message-ipc to master Oct 9, 2019
@miniak miniak removed the wip label Oct 9, 2019
@miniak miniak requested a review from nornagon Oct 9, 2019
@miniak miniak marked this pull request as ready for review Oct 9, 2019
@miniak

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

@nornagon can you please review? I am planning to address error / exception serialization separately. There is some duplicate code there.

@miniak miniak requested review from deepak1556 and zcbenz Oct 9, 2019
lib/browser/remote/server.ts Outdated Show resolved Hide resolved
lib/browser/remote/server.ts Show resolved Hide resolved
@miniak miniak force-pushed the miniak/simplify-remote branch 2 times, most recently from 88a2e1d to 11d9444 Oct 9, 2019
@miniak miniak added the wip label Oct 9, 2019
@miniak

This comment has been minimized.

Copy link
Contributor Author

commented Oct 9, 2019

@nornagon do we mind loosing the from property indicating which process the Error came from, which was added by the old errorUtils serializer?

@miniak miniak force-pushed the miniak/simplify-remote branch from 11d9444 to f4e474e Oct 9, 2019
String,
Date,
RegExp,
ArrayBuffer

This comment has been minimized.

Copy link
@nornagon

nornagon Oct 9, 2019

Member

Map, Set?

This comment has been minimized.

Copy link
@miniak

miniak Oct 9, 2019

Author Contributor

those can contain other objects, which require custom serialization.

Copy link
Member

left a comment

nice cleanup. remote is still a jungle but at least it's trending less jungle-y!

@@ -43,17 +41,17 @@ function wrapArgs (args, visited = new Set()) {
}
visited.delete(value)

This comment has been minimized.

Copy link
@nornagon

nornagon Oct 9, 2019

Member

it's still a mystery to me why we have two separate serialization algorithms

This comment has been minimized.

Copy link
@miniak

miniak Oct 9, 2019

Author Contributor

do you mean main -> renderer and renderer -> main?

This comment has been minimized.

Copy link
@nornagon
@miniak miniak removed the wip label Oct 9, 2019
@jkleinsc jkleinsc merged commit b92163d into master Oct 10, 2019
15 checks passed
15 checks passed
Artifact Comparison No Changes
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-woa-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20191009.22 succeeded
Details
electron-arm64-testing Build #20191009.24 succeeded
Details
electron-woa-testing Build #20191009.18 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Oct 10, 2019

Release Notes Persisted

The remote module now properly serializes Boolean, Number, String and RegExp instances.

@jkleinsc jkleinsc deleted the miniak/simplify-remote branch Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.