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

Incorrect semantics for globals #146

Closed
jlongster opened this issue Jan 8, 2022 · 7 comments
Closed

Incorrect semantics for globals #146

jlongster opened this issue Jan 8, 2022 · 7 comments
Labels
behaviour mismatch Different behaviour to Workers runtime fixed-in-next Fixed in next release

Comments

@jlongster
Copy link

These should be true:

{}.constructor === Object
[].constructor === Array

They are true in node, browser, and are true in production cloudflare pages/workers. They are not true in miniflare.

(my use case is trying to use ClojureScript compiled code which is using these)

@mrbbot
Copy link
Contributor

mrbbot commented Jan 8, 2022

Hey! 👋 Which version of miniflare are you using? This should be fixed in version 2.0.0. 😕

@jlongster
Copy link
Author

I was just looking into it and commenting out the line which proxies globals fixes it, and I don't see that code anymore on github! The newer code for working with globals looks more reasonable and if it's released, should fix it! Will try it out. Thanks a lot! I was worried this would be a weird esoteric thing but looks like it's straight-forward and already being addressed :)

@jlongster
Copy link
Author

jlongster commented Jan 8, 2022

I am using version 0.0.0-8ab7d2e of wrangler (because it has some fixes for watch mode) which includes version 2.0.0-rc.4 of miniflare. Is the code on github released? I'm still seeing this code in runner-vm in 2.0.0-rc.4:

function proxyHasInstance(target, hasInstance, handler) {
  return new Proxy(target, {
    get(target2, property, receiver) {
      if (property === Symbol.hasInstance)
        return hasInstance;
      return Reflect.get(target2, property, receiver);
    },
    ...handler
  });
}
function makeProxiedGlobals(blockCodeGeneration) {
  if (!blockCodeGeneration)
    blockCodeGeneration = void 0;
  return {
    Object: proxyHasInstance(Object, isObject),
    Array: proxyHasInstance(Array, Array.isArray),
    Promise: proxyHasInstance(Promise, import_util.types.isPromise),
    RegExp: proxyHasInstance(RegExp, import_util.types.isRegExp),
    Error: proxyHasInstance(Error, isError(Error)),
    EvalError: proxyHasInstance(EvalError, isError(EvalError)),
    RangeError: proxyHasInstance(RangeError, isError(RangeError)),
    ReferenceError: proxyHasInstance(ReferenceError, isError(ReferenceError)),
    SyntaxError: proxyHasInstance(SyntaxError, isError(SyntaxError)),
    TypeError: proxyHasInstance(TypeError, isError(TypeError)),
    URIError: proxyHasInstance(URIError, isError(URIError)),
    Function: proxyHasInstance(Function, isFunction, blockCodeGeneration && {
      construct() {
        throw new EvalError("Code generation from strings disallowed for this context");
      }
    })
  };
}

@mrbbot
Copy link
Contributor

mrbbot commented Jan 8, 2022

I've just created the PR to upgrade to 2.0.0 here: cloudflare/workers-sdk#221. The most recently published version of wrangler should be using 2.0.0-rc.5 which disables this proxying stuff by default, but doesn't have the correct cross-realm instanceof behaviour.

@mrbbot mrbbot added behaviour mismatch Different behaviour to Workers runtime fixed-in-next Fixed in next release labels Jan 8, 2022
@jlongster
Copy link
Author

thanks!

doesn't have the correct cross-realm instanceof behaviour

what constitutes a realm here? what real-world use cases would see this problem? I'm assume each module isn't a realm as that would mean object passed across them wouldn't work. where are the realm boundaries?

@mrbbot
Copy link
Contributor

mrbbot commented Jan 8, 2022

The comment at the start of this file goes into detail on this. 🙂 The realm boundary is between stuff running in Node (runtime APIs: KV, etc) and stuff running in Miniflare's vm sandbox (user code).

@jlongster
Copy link
Author

cool, that's what I figured but wanted to make sure that was right. Shouldn't be a problem for my case, and that tradeoff makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behaviour mismatch Different behaviour to Workers runtime fixed-in-next Fixed in next release
Projects
None yet
Development

No branches or pull requests

2 participants