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 environment detection in WeakMapPolyfill #8

Closed
wbuchwalter opened this issue Jun 19, 2015 · 10 comments
Closed

Incorrect environment detection in WeakMapPolyfill #8

wbuchwalter opened this issue Jun 19, 2015 · 10 comments
Milestone

Comments

@wbuchwalter
Copy link
Contributor

In CreateWeakMapPolyfill(), to detect if we are running under node, this code is used:

const isNode = typeof global !== "undefined" &&
  typeof module === "object" &&
  typeof module.exports === "object" &&
  typeof require === "function";

But when using tools which implement CommonJS API in the browser such as Webpack or Browserify, isNode will be incorrectly set to true.

We could replace the check by this one:

 const isNode = Object.prototype.toString.call(global.process) === '[object process]';

Related PR: #7

Thanks!

@vinodsantharam
Copy link

+1

1 similar comment
@GuillaumeSalles
Copy link

+1

@sectore
Copy link

sectore commented Jul 14, 2015

Same issue by using jspm.

Behind the scenes SystemJS builder adds an @empty to var nodeCrypto = isNode && require('@empty'); (Reflect.ts#L1351) for using an empty function.

So the check of nodeCrypto (Reflect.ts#L1399) is failed:
Error evaluating XXX/jspm_packages/npm/reflect-metadata@0.1.0/Reflect.js TypeError: 'undefined' is not a function (evaluating 'nodeCrypto.randomBytes(size)')

These issue can be fixed with #7 by @wbuchwalter as well.

-Jens

@wbuchwalter
Copy link
Contributor Author

Hey @rbuckton , any feedback on this? Thanks!

@tamascsaba
Copy link

@rbuckton ????

@rbuckton
Copy link
Owner

My apologies, I've been out of the office for the past few weeks. I'm also clarifying any CLA requirements for this project before I can accept any pull requests.

@PatrickJS
Copy link

👍

@rbuckton
Copy link
Owner

Closed #8 via #7.

@rbuckton rbuckton added this to the v0.1.1 milestone Aug 26, 2015
@rolandjitsu
Copy link

@rbuckton how soon will this be released?

@rbuckton
Copy link
Owner

rbuckton commented Sep 3, 2015

v0.1.1 has been published to npm.

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

8 participants