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

Objects from imported modules have different core types, making instanceof useless #16

Open
phorks opened this issue Jul 13, 2022 · 4 comments

Comments

@phorks
Copy link

phorks commented Jul 13, 2022

The instanceof operator always returns false when used with the values from imported modules.

const moduleFromString = require("module-from-string");

const code = "throw new Error();";

try {
  const x = moduleFromString.requireFromString(code);
} catch (err) {
  console.log(Object.getPrototypeOf(err).name === Error.prototype.name);
  console.log(err instanceof Error);
  console.log(err instanceof Object);
}

It outputs:

  • true
  • false
  • false

Or:

const moduleFromString = require("module-from-string");

const code = "exports.obj = {}";

const x = moduleFromString.requireFromString(code);
console.log(Object.getPrototypeOf(x.obj).name === Object.prototype.name);
console.log(x.obj instanceof Object);

Outputs:

  • true
  • false

Using importFromString yields the same results. It's a side effect of using runInNewContext instead of runInThisContext (or providing context in case of the vm.Module). If this behavior is not intentional, I would be glad to fix it.

@exuanbo
Copy link
Owner

exuanbo commented Jul 13, 2022

This is intentional, but I have added an option useCurrentGlobal and some factory functions createRequireFromString etc. in the pre-released version 3.2.0-0. You can install it and give it a try.

If it works well, I will update the README and publish a new minor version tomorrow.

@exuanbo
Copy link
Owner

exuanbo commented Jul 17, 2022

Please use the newly added option useCurrentGlobal in 3.2.0.

@exuanbo exuanbo closed this as completed Jul 17, 2022
@phorks
Copy link
Author

phorks commented Jul 17, 2022

Thank you. Just a question about the implementation though, did you consider the compileFunction approach that the default nodejs cjs loader takes (here)? That way, there would be no need to shallow copy the entire global object each time.

@exuanbo
Copy link
Owner

exuanbo commented Jul 17, 2022

Thank you for the suggestion, I haven't actually considered this approach.

I will try refactoring the code in the next few days.

@exuanbo exuanbo reopened this Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants