The Wayback Machine - https://web.archive.org/web/20201029215931/https://github.com/electron/electron/pull/24301
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

fix: do not register the node ESM loader in renderer processes #24301

Merged
merged 3 commits into from Jul 16, 2020
Merged

Conversation

@MarshallOfSound
Copy link
Member

@MarshallOfSound MarshallOfSound commented Jun 25, 2020

Only one ESM loader can be registered per isolate, in renderer processes this should be blink. This patches node so that it won't register its handler (overriding blinks) in non-browser processes.

Notes: no-notes

@MarshallOfSound MarshallOfSound requested a review from electron/wg-upgrades as a code owner Jun 25, 2020
@MarshallOfSound MarshallOfSound requested a review from codebytere Jun 25, 2020
@deepak1556
Copy link
Member

@deepak1556 deepak1556 commented Jun 25, 2020

Is it possible to add test for this change ?

@codebytere
Copy link
Member

@codebytere codebytere commented Jun 25, 2020

@deepak1556 and @MarshallOfSound i think yes - we added the sandbox change to ARM to account for this here, so we should be able to remove it now. If it fails, we know this didn't have the intended effect.

@deepak1556
Copy link
Member

@deepak1556 deepak1556 commented Jun 25, 2020

Failing vanilla node tests are related.

@codebytere
Copy link
Member

@codebytere codebytere commented Jun 25, 2020

oh boy missed those - i think we'll need to ensure it's enabled in run as node mode @MarshallOfSound

@codebytere
Copy link
Member

@codebytere codebytere commented Jun 29, 2020

@MarshallOfSound can you rebase this against the accepted upstream solution: nodejs/node#34060

@codebytere codebytere force-pushed the fix-esm branch from f396bff to b55147b Jul 15, 2020
@codebytere codebytere added the wip label Jul 15, 2020
@codebytere codebytere changed the title fix: do not register the node ESM loader in renderer processes [wip] fix: do not register the node ESM loader in renderer processes Jul 15, 2020
@codebytere codebytere force-pushed the fix-esm branch from b55147b to 88413e6 Jul 15, 2020
@codebytere codebytere changed the title [wip] fix: do not register the node ESM loader in renderer processes fix: do not register the node ESM loader in renderer processes Jul 15, 2020
@codebytere codebytere removed the wip label Jul 15, 2020
@codebytere codebytere force-pushed the fix-esm branch from 88413e6 to debb4bf Jul 15, 2020
@codebytere codebytere force-pushed the fix-esm branch from debb4bf to 7212687 Jul 15, 2020
@codebytere codebytere mentioned this pull request Jul 16, 2020
5 of 5 tasks complete
@codebytere codebytere merged commit d8be385 into master Jul 16, 2020
13 of 15 checks passed
13 of 15 checks passed
appveyor: win-x64-testing AppVeyor build failed
Details
electron-woa-testing Build #20200715.12 had test failures
Details
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-pr AppVeyor build succeeded
Details
build-linux Workflow: build-linux
Details
build-mac Workflow: build-mac
Details
electron-arm-testing Build #20200715.31 succeeded
Details
electron-arm64-testing Build #20200715.31 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk
Copy link

@release-clerk release-clerk bot commented Jul 16, 2020

No Release Notes

@codebytere codebytere deleted the fix-esm branch Jul 16, 2020
georgexu99 pushed a commit to georgexu99/electron that referenced this pull request Jul 28, 2020
georgexu99 pushed a commit to georgexu99/electron that referenced this pull request Jul 28, 2020
sentialx added a commit to sentialx/electron that referenced this pull request Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.