feat: I guess it's esm (#37535)

* fix: allow ESM loads from within ASAR files

* fix: ensure that ESM entry points finish loading before app ready

* fix: allow loading ESM entrypoints via default_app

* fix: allow ESM loading for renderer preloads

* docs: document current known limitations of esm

* chore: add patches to support blending esm handlers

* refactor: use SetDefersLoading instead of JoinAppCode in renderers

Blink has it's own event loop so pumping the uv loop in the renderer is not enough, luckily in blink we can suspend the loading of the frame while we do additional work.

* chore: add patch to expose SetDefersLoading

* fix: use fileURLToPath instead of pathname

* chore: update per PR feedback

* fix: fs.exists/existsSync should never throw

* fix: convert path to file url before importing

* fix: oops

* fix: oops

* Update docs/tutorial/esm-limitations.md

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* windows...

* windows...

* chore: update patches

* spec: fix tests and document empty body edge case

* Apply suggestions from code review

Co-authored-by: Daniel Scalzi <d_scalzi@yahoo.com>
Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* spec: add tests for esm

* spec: windows

* chore: update per PR feedback

* chore: update patches

* Update shell/common/node_bindings.h

Co-authored-by: Jeremy Rose <jeremya@chromium.org>

* chore: update patches

* rebase

* use cjs loader by default for preload scripts

* chore: fix lint

* chore: update patches

* chore: update patches

* chore: fix patches

* build: debug depshash

* ?

* Revert "build: debug depshash"

This reverts commit 0de82523fb93f475226356b37418ce4b69acdcdf.

* chore: allow electron as builtin protocol in esm loader

* Revert "Revert "build: debug depshash""

This reverts commit ff86b1243ca6d05c9b3b38e0a6d717fb380343a4.

* chore: fix esm doc

* chore: update node patches

---------

Co-authored-by: Jeremy Rose <jeremya@chromium.org>
Co-authored-by: electron-patch-conflict-fixer[bot] <83340002+electron-patch-conflict-fixer[bot]@users.noreply.github.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Daniel Scalzi <d_scalzi@yahoo.com>
This commit is contained in:
Samuel Attard 2023-08-30 17:38:07 -07:00 committed by GitHub
parent b8ac798344
commit ac031bf8de
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
36 changed files with 910 additions and 57 deletions

View file

@ -28,6 +28,8 @@ fix_expose_the_built-in_electron_module_via_the_esm_loader.patch
api_pass_oomdetails_to_oomerrorcallback.patch
fix_expose_lookupandcompile_with_parameters.patch
enable_crashpad_linux_node_processes.patch
fix_lazyload_fs_in_esm_loaders_to_apply_asar_patches.patch
chore_expose_importmoduledynamically_and.patch
test_formally_mark_some_tests_as_flaky.patch
fix_adapt_debugger_tests_for_upstream_v8_changes.patch
chore_remove_--no-harmony-atomics_related_code.patch

View file

@ -0,0 +1,103 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <marshallofsound@electronjs.org>
Date: Wed, 8 Mar 2023 13:02:17 -0800
Subject: chore: expose ImportModuleDynamically and
HostInitializeImportMetaObjectCallback to embedders
This also subtly changes the behavior of shouldNotRegisterESMLoader to ensure that node sets up the handlers
internally but simply avoids setting its own handlers on the Isolate. This is so that Electron can set it to
its own blended handler between Node and Blink.
Not upstreamable.
diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js
index f96d19969aa59a9964d947a9fd6295cf25ad3b03..15116b78f82c977bba67ce98ce57232500dfaef8 100644
--- a/lib/internal/process/pre_execution.js
+++ b/lib/internal/process/pre_execution.js
@@ -554,7 +554,7 @@ function initializeESMLoader() {
// Create this WeakMap in js-land because V8 has no C++ API for WeakMap.
internalBinding('module_wrap').callbackMap = new SafeWeakMap();
- if (getEmbedderOptions().shouldNotRegisterESMLoader) return;
+ const shouldSetOnIsolate = !getEmbedderOptions().shouldNotRegisterESMLoader;
const {
setImportModuleDynamicallyCallback,
@@ -563,8 +563,8 @@ function initializeESMLoader() {
const esm = require('internal/process/esm_loader');
// Setup per-isolate callbacks that locate data or callbacks that we keep
// track of for different ESM modules.
- setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject);
- setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback);
+ setInitializeImportMetaObjectCallback(esm.initializeImportMetaObject, shouldSetOnIsolate);
+ setImportModuleDynamicallyCallback(esm.importModuleDynamicallyCallback, shouldSetOnIsolate);
// Patch the vm module when --experimental-vm-modules is on.
// Please update the comments in vm.js when this block changes.
diff --git a/src/module_wrap.cc b/src/module_wrap.cc
index 0645b3ddf506df2a76f5661f0ec6bb35d5d8b94e..e0f1b2d51f3055b2250f2c0dc1dfd1048b645dd9 100644
--- a/src/module_wrap.cc
+++ b/src/module_wrap.cc
@@ -547,7 +547,7 @@ MaybeLocal<Module> ModuleWrap::ResolveModuleCallback(
return module->module_.Get(isolate);
}
-static MaybeLocal<Promise> ImportModuleDynamically(
+MaybeLocal<Promise> ImportModuleDynamically(
Local<Context> context,
Local<v8::Data> host_defined_options,
Local<Value> resource_name,
@@ -629,12 +629,13 @@ void ModuleWrap::SetImportModuleDynamicallyCallback(
Environment* env = Environment::GetCurrent(args);
HandleScope handle_scope(isolate);
- CHECK_EQ(args.Length(), 1);
+ CHECK_EQ(args.Length(), 2);
CHECK(args[0]->IsFunction());
Local<Function> import_callback = args[0].As<Function>();
env->set_host_import_module_dynamically_callback(import_callback);
- isolate->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically);
+ if (args[1]->IsBoolean() && args[1]->BooleanValue(isolate))
+ isolate->SetHostImportModuleDynamicallyCallback(ImportModuleDynamically);
}
void ModuleWrap::HostInitializeImportMetaObjectCallback(
@@ -665,13 +666,14 @@ void ModuleWrap::SetInitializeImportMetaObjectCallback(
Environment* env = Environment::GetCurrent(args);
Isolate* isolate = env->isolate();
- CHECK_EQ(args.Length(), 1);
+ CHECK_EQ(args.Length(), 2);
CHECK(args[0]->IsFunction());
Local<Function> import_meta_callback = args[0].As<Function>();
env->set_host_initialize_import_meta_object_callback(import_meta_callback);
- isolate->SetHostInitializeImportMetaObjectCallback(
- HostInitializeImportMetaObjectCallback);
+ if (args[1]->IsBoolean() && args[1]->BooleanValue(isolate))
+ isolate->SetHostInitializeImportMetaObjectCallback(
+ HostInitializeImportMetaObjectCallback);
}
MaybeLocal<Value> ModuleWrap::SyntheticModuleEvaluationStepsCallback(
diff --git a/src/module_wrap.h b/src/module_wrap.h
index 58b233d036515c52d9bd5574c776c2ea65d2ecb1..5f7ef75480a76761c6fa62061c8700c812a3fc6f 100644
--- a/src/module_wrap.h
+++ b/src/module_wrap.h
@@ -30,7 +30,14 @@ enum HostDefinedOptions : int {
kLength = 10,
};
-class ModuleWrap : public BaseObject {
+NODE_EXTERN v8::MaybeLocal<v8::Promise> ImportModuleDynamically(
+ v8::Local<v8::Context> context,
+ v8::Local<v8::Data> host_defined_options,
+ v8::Local<v8::Value> resource_name,
+ v8::Local<v8::String> specifier,
+ v8::Local<v8::FixedArray> import_assertions);
+
+class NODE_EXTERN ModuleWrap : public BaseObject {
public:
enum InternalFields {
kModuleWrapBaseField = BaseObject::kInternalFieldCount,

View file

@ -11,7 +11,7 @@ trying to see whether or not the lines are greyed out. One possibility
would be to upstream a changed test that doesn't hardcode line numbers.
diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot
index 0334a0b4faa3633aa8617b9538873e7f3540513b..28ba4d18fe5e3caf4d904a055550110fd4faa609 100644
index 0334a0b4faa3633aa8617b9538873e7f3540513b..0300d397c6a5b82ef2d0feafca5f8bd746b1f5b0 100644
--- a/test/fixtures/errors/force_colors.snapshot
+++ b/test/fixtures/errors/force_colors.snapshot
@@ -4,11 +4,12 @@ throw new Error('Should include grayed stack trace')
@ -27,7 +27,7 @@ index 0334a0b4faa3633aa8617b9538873e7f3540513b..28ba4d18fe5e3caf4d904a055550110f
+ at Object..js (node:internal*modules*cjs*loader:1326:10)
+ at Module.load (node:internal*modules*cjs*loader:1126:32)
+ at node:internal*modules*cjs*loader:967:12
+ at Function._load (node:electron*js2c*asar_bundle:756:32)
+ at Function._load (node:electron*js2c*asar_bundle:776:32)
+ at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:96:12)
 at node:internal*main*run_main_module:23:47

View file

@ -17,6 +17,27 @@ index 219ef03a21214deb8961044cfc18ef9c1e711b60..7749b37001f869fe565d8c450ff7ca2b
};
/**
diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js
index 1178f06074ec8ec02a2e7b5cf8cbe7e68b72f809..d64d4452a50bc4469b2d2fcc3251b3df31cda6ec 100644
--- a/lib/internal/modules/esm/load.js
+++ b/lib/internal/modules/esm/load.js
@@ -116,6 +116,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
protocol !== 'file:' &&
protocol !== 'data:' &&
protocol !== 'node:' &&
+ protocol !== 'electron:' &&
(
!experimentalNetworkImports ||
(
@@ -124,7 +125,7 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) {
)
)
) {
- const schemes = ['file', 'data', 'node'];
+ const schemes = ['file', 'data', 'node', 'electron'];
if (experimentalNetworkImports) {
ArrayPrototypePush(schemes, 'https', 'http');
}
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index 2cac6f2d450fab014544d15439e51575f86ccb38..ce2d851da2577e6e99980eb75337f629b38fddbf 100644
--- a/lib/internal/modules/esm/resolve.js

View file

@ -0,0 +1,103 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Samuel Attard <marshallofsound@electronjs.org>
Date: Tue, 7 Mar 2023 16:17:41 -0800
Subject: fix: lazyload fs in esm loaders to apply asar patches
Changes { foo } from fs to just "fs.foo" so that our patching of fs is applied to esm loaders
diff --git a/lib/internal/modules/esm/load.js b/lib/internal/modules/esm/load.js
index d64d4452a50bc4469b2d2fcc3251b3df31cda6ec..8d8f1b24fb68cce2b37e43f92aac200b4ee73cea 100644
--- a/lib/internal/modules/esm/load.js
+++ b/lib/internal/modules/esm/load.js
@@ -20,7 +20,7 @@ const experimentalNetworkImports =
const { Buffer: { from: BufferFrom } } = require('buffer');
-const { readFile: readFileAsync } = require('internal/fs/promises').exports;
+const fs = require('fs');
const { URL } = require('internal/url');
const {
ERR_INVALID_URL,
@@ -34,7 +34,7 @@ async function getSource(url, context) {
let responseURL = url;
let source;
if (parsed.protocol === 'file:') {
- source = await readFileAsync(parsed);
+ source = await fs.promises.readFile(parsed);
} else if (parsed.protocol === 'data:') {
const match = RegExpPrototypeExec(DATA_URL_PATTERN, parsed.pathname);
if (!match) {
diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js
index ce2d851da2577e6e99980eb75337f629b38fddbf..fed26d25b59d66ab6e5160e2e13c8eea0cb44f8d 100644
--- a/lib/internal/modules/esm/resolve.js
+++ b/lib/internal/modules/esm/resolve.js
@@ -26,11 +26,7 @@ const {
} = primordials;
const internalFS = require('internal/fs/utils');
const { BuiltinModule } = require('internal/bootstrap/loaders');
-const {
- realpathSync,
- statSync,
- Stats,
-} = require('fs');
+const fs = require('fs');
const { getOptionValue } = require('internal/options');
const pendingDeprecation = getOptionValue('--pending-deprecation');
// Do not eagerly grab .manifest, it may be in TDZ
@@ -171,14 +167,14 @@ const realpathCache = new SafeMap();
* @returns {import('fs').Stats}
*/
const tryStatSync =
- (path) => statSync(path, { throwIfNoEntry: false }) ?? new Stats();
+ (path) => fs.statSync(path, { throwIfNoEntry: false }) ?? new fs.Stats();
/**
* @param {string | URL} url
* @returns {boolean}
*/
function fileExists(url) {
- return statSync(url, { throwIfNoEntry: false })?.isFile() ?? false;
+ return fs.statSync(url, { throwIfNoEntry: false })?.isFile() ?? false;
}
/**
@@ -326,7 +322,7 @@ function finalizeResolution(resolved, base, preserveSymlinks) {
}
if (!preserveSymlinks) {
- const real = realpathSync(path, {
+ const real = fs.realpathSync(path, {
[internalFS.realpathCacheKey]: realpathCache,
});
const { search, hash } = resolved;
diff --git a/lib/internal/modules/esm/translators.js b/lib/internal/modules/esm/translators.js
index 1ceb89da21610c703f4a18be5888373c7feaa370..347558c805c8ecd3f7ff4f6324ef7df68badc52f 100644
--- a/lib/internal/modules/esm/translators.js
+++ b/lib/internal/modules/esm/translators.js
@@ -24,7 +24,7 @@ function lazyTypes() {
return _TYPES = require('internal/util/types');
}
-const { readFileSync } = require('fs');
+const fs = require('fs');
const { extname, isAbsolute } = require('path');
const {
hasEsmSyntax,
@@ -131,7 +131,7 @@ translators.set('module', async function moduleStrategy(url, source, isMain) {
*/
function enrichCJSError(err, content, filename) {
if (err != null && ObjectGetPrototypeOf(err) === SyntaxErrorPrototype &&
- hasEsmSyntax(content || readFileSync(filename, 'utf-8'))) {
+ hasEsmSyntax(content || fs.readFileSync(filename, 'utf-8'))) {
// Emit the warning synchronously because we are in the middle of handling
// a SyntaxError that will throw and likely terminate the process before an
// asynchronous warning would be emitted.
@@ -207,7 +207,7 @@ function cjsPreparseModuleExports(filename) {
let source;
try {
- source = readFileSync(filename, 'utf8');
+ source = fs.readFileSync(filename, 'utf8');
} catch {
// Continue regardless of error.
}