Skip to content

Conversation

andybee
Copy link
Contributor

@andybee andybee commented Apr 21, 2025

This fix addresses an issue with importing mocks in an MJS file after the Node 20.19 back port of require(esm).

Uses the process.features.require_module flag to work out what should be happening.

As there were no existing tests to validate the mock import behaviour, I was unsure how to create a test for this.

@filmaj
Copy link
Member

filmaj commented Apr 26, 2025

How can I manually test this? Since the require_module API was added in node 23 according to the docs, not sure how to validate things are working? Does this add support for plugins written in MJS, or arc-app-lambdas written in MJS?

If you can tell me how I can reproduce locally to: show the failure (I'm not 100% clear on what the issue is), then show how this PR fixes it, then I think that will clear up my question,

@andybee
Copy link
Contributor Author

andybee commented Apr 26, 2025

Good question. So this is specifically to do with the functionality that imports mocks that set the event payload when invoking a Lambda. Prior to this bug fix, running Node <= 20.18, you could have a file called sandbox-invoke-mocks.mjs and it would work as you expect - hit I, choose a Lambda that you have a specific mock setup for, get the list of mocks available, pick one and it invokes with that chosen mock.

When we hit Node 20.19, which changed the require() behaviour, it wouldn't error on start up, but hitting i and choosing a Lambda, the invocation happened immediately - the same behaviour as if having no mocks at all. Under the hood this was because of the way the import came back - very similar issue to what was happening in inventory before Ryan's fix.

This borrows the same kind of fix but specifically for mocks, but uses [process.features.require_module](https://nodejs.org/api/process.html#processfeaturesrequire_module) flag rather than sniffing the Node version.

Copy link
Member

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Finally got around to this 😅

Validated this works well!

@filmaj filmaj merged commit 2aafded into architect:main Jun 26, 2025
11 checks passed
@filmaj
Copy link
Member

filmaj commented Jun 26, 2025

Should be live shortly as 2.0.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants