-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(functions,emulators): loadTriggers() with top level await #8394
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
base: master
Are you sure you want to change the base?
Conversation
Context: ESM functions with firestore triggers The expected behavior of `loadTriggers()` is to try and `require()` the function entrypoint, falling back to dynamic `import()` when node raises a ERR_REQUIRE_ESM error. But node may also raise an ERR_REQUIRE_ASYNC_MODULE error when the module has top level awaits This commit simply makes it so ERR_REQUIRE_ASYNC_MODULE also falls back to `import()`
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
This is great contribution 🎉 . Think we can add tests later when functions bring support for Node24. |
@@ -21,7 +21,7 @@ async function loadModule(packageDir) { | |||
try { | |||
return require(packageDir); | |||
} catch (e) { | |||
if (e.code === "ERR_REQUIRE_ESM") { | |||
if (e.code === "ERR_REQUIRE_ESM" && e.code !== "ERR_REQUIRE_ASYNC_MODULE") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition seems wrong. e.code !== "ERR_REQUIRE_ASYNC_MODULE"
is always true if e.code === "ERR_REQUIRE_ESM"
.
Description
Context: ESM functions with firestore triggers
The expected behavior of
loadTriggers()
is totry and
require()
the function entrypoint,falling back to dynamic
import()
when node raises a ERR_REQUIRE_ESM error. But node may also raise an ERR_REQUIRE_ASYNC_MODULE error when the module has top level awaitsThis commit simply makes it so ERR_REQUIRE_ASYNC_MODULE also falls back to
import()
Scenarios Tested
Tested on our (private) repo
Let me know if tests need to be added/updated :)