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

rfc: Preload Realm for Service Workers #8

Merged
merged 4 commits into from
Sep 20, 2024

Conversation

samuelmaddock
Copy link
Member

@samuelmaddock samuelmaddock commented Aug 6, 2024

Reopened from #4 as my fork somehow became unlinked

Preload realms provide an isolated context to run JavaScript prior to code evaluating in another
context. This expands functionality of Electron's preload scripts to provide new context targets.
This proposal intends to target Service Worker contexts with potential plans for new contexts (web
workers, shared workers) in future proposals.

Read rendered document

Recommended prerequisite readings:

If you're curious, there's a very rough wip prototype being written in https://github.com/samuelmaddock/electron/tree/feat/preload-realm


Todo:

  • Try to get a better understanding of where ShadowRealm is at in the TC39 process.
  • Call out the option of landing the API as experimental until ShadowRealm is stable

more

more details on ServiceWorkerMain

preload realm scripts

add more to rationale

format and autowrap

clarify questions

clarifications

move rfc

update rfc metadata

prefer ServiceWorkerMain.ipc over ipcMain

drop 'initiator' terminology in favor of reusing 'main' world

spelling
@samuelmaddock samuelmaddock requested a review from a team as a code owner August 6, 2024 14:27
@samuelmaddock samuelmaddock added the pending-review Waiting for reviewers to give feedback on the PR specifications label Aug 6, 2024
@samuelmaddock
Copy link
Member Author

Recent changes:

  • ipcMain no longer receives SW IPC messages in favor of handling them on ServiceWorkerMain.ipc.
  • "Main world" is now used by contextBridge instead of the added "initiator world" to simplify reusing preload scripts between frames and service workers.

Copy link
Member

@itsananderson itsananderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the ShadowRealms proposal is apparently still in stage 2.7, we should make sure we mark this API as experimental until the web platform API is fully locked in, but otherwise this RFC LGTM

Copy link
Member

@erickzhao erickzhao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC LGTM

text/0002-preload-realm.md Outdated Show resolved Hide resolved
Copy link
Member

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RFC LGTM

@samuelmaddock
Copy link
Member Author

samuelmaddock commented Sep 11, 2024

Regarding ShadowRealm TC39 proposal status, there's been a couple updates this year.

Before the ShadowRealm proposal is able to move forward, more work is needed on confirming the selection of APIs to be included within the new realm. Also, more test coverage is needed with WPT mentioned.

The good news is that the V8 implementation seems to be stable according to the February slides and I added a WPT test as part of this RFC :)

Assuming no major changes regarding Blink's implementation, we may see more built-in APIs exposed within the ShadowRealm which would be beneficial for Preload Realms as well.

@erickzhao erickzhao added final-comment-period and removed pending-review Waiting for reviewers to give feedback on the PR specifications labels Sep 19, 2024
@samuelmaddock samuelmaddock merged commit e0660f0 into electron:main Sep 20, 2024
2 checks passed
@samuelmaddock samuelmaddock deleted the preload-realms-rfc branch September 20, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants