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

JavaScript vm broken (bad Proxy implementation?) #309

Closed
freehuntx opened this issue Aug 25, 2021 · 5 comments
Closed

JavaScript vm broken (bad Proxy implementation?) #309

freehuntx opened this issue Aug 25, 2021 · 5 comments
Labels

Comments

@freehuntx
Copy link

freehuntx commented Aug 25, 2021

Describe the bug

When defining properties on a object returned by the vm, they behave differently than in NodeJS.
This breaks for example the testing framework Jest. (As you can see here: #293)

Example code:

const { runInContext, createContext } = require('vm');

const context = createContext();
const vmGlobal = runInContext('this', context);

Object.defineProperties(vmGlobal, {
  works: {
    configurable: true,
    value: 'This will work'
  },
  worksNot: {
    configurable: false,
    value: 'This will not work (in webcontainer)'
  }
});

console.log(vmGlobal.works); // success
console.log(vmGlobal.worksNot); // error
Object.keys(vmGlobal); // error

Link to the blitz that caused the error

https://stackblitz.com/edit/node-7mnp6x

Steps to reproduce

  1. Open the linked stackblitz
  2. run node index in the console
  3. See the error

Expected behavior

Console should print:

This will work
This will not work (in webcontainer)

Screenshots

image

Platform

Browser name  = Chrome 
Full version  = 92.0.4515.159
Major version = 92
navigator.appName = Netscape
navigator.userAgent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.159 Safari/537.36
performance.memory = {
  "totalJSHeapSize": 96025376,
  "usedJSHeapSize": 90858908,
  "jsHeapSizeLimit": 4294705152
}
Hash = 8ffc536d8181e8b30c228dec423d18afae547c40

Additional context

I created this bug ticket as a result of this: #293

@freehuntx
Copy link
Author

Maybe the issue lays somewhere else. Take this script for example:

const { runInContext, createContext } = require('vm');

console.log(runInContext('this', createContext()));
console.log(runInContext('global', createContext()));

In stackblitz this works, in native NodeJS not. Since global should not be defined. Also "this" seems to be different than in native NodeJS.

@d3lm d3lm added the tracked label Aug 30, 2021
@d3lm
Copy link

d3lm commented Aug 30, 2021

Thanks for reporting this issue. It seems to be a bug.

@d3lm
Copy link

d3lm commented Aug 31, 2021

Good news, this has been fixed and it's now available. Here's an example. I have also tested jest@27 and it seems to work. Maybe give it another whirl and let us how it goes. Please create a new issue if there's still a problem.

@d3lm d3lm closed this as completed Aug 31, 2021
@freehuntx
Copy link
Author

You guys are awesome! It works, thanks!

@d3lm
Copy link

d3lm commented Sep 1, 2021

@freehuntx Just a quick update that vm.runInContext('global', vm.createContext()) won't behave the same as on local. I had to change this again because the way I fixed it does not work in all cases and typeof global was also throwing a reference error. Unfortunately, this is a limitation of WebContainer's vm module. The closest I can get is to return undefined if you try to evaluate global and the context doesn't define it. It's a bummer but sadly a hard limitation. But I'd say this is an edge case, and the rest still works as expected.

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

No branches or pull requests

2 participants