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.

Sign up for GitHub

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

Jump to bottom

Enabling nodeIntegration + Electron 27 #367

Merged
merged 8 commits into from
Apr 29, 2024
Merged

Enabling nodeIntegration + Electron 27 #367

merged 8 commits into from
Apr 29, 2024

Conversation

viktor44
Copy link
Collaborator

What is this PR

  1. nodeIntegration is enabled for Webview.
  2. contextIsolation is enabled for Webview. It is necessary because otherwise otherwise huge part of apps don't work.
  3. as a result of (2), integration with window.bx is rewritten with contextBridge (see)
  4. Electron is upgraded up to 27 (maybe it's better to move to separate PR, but it doesn't require any changes in code now)

Thank you @magne4000 for research in PR

@viktor44 viktor44 requested a review from magne4000 April 28, 2024 06:48
magne4000
magne4000 requested changes Apr 28, 2024
View reviewed changes
Copy link
Member

@magne4000 magne4000 left a comment

Choose a reason for hiding this comment

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

I'm not sure why sendSubscribeToProxy has been replaced by addListenerToChannel, could you explain?

Also, you can remove old code comments if they have been replaced by new code.

packages/app/src/plugins/bxapi.d.ts Outdated Show resolved Hide resolved
packages/app/src/plugins/webview-preload.js Show resolved Hide resolved
packages/app/src/webui/preload.js Show resolved Hide resolved
@magne4000
Copy link
Member

LGTM, nice works!

@viktor44 viktor44 merged commit 173c630 into getstation:main Apr 29, 2024
3 checks passed
@viktor44 viktor44 deleted the feature/context_isolation branch August 25, 2024 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@magne4000 magne4000 magne4000 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@viktor44 @magne4000