-
Notifications
You must be signed in to change notification settings - Fork 12
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
Random.secure() does not work in browser after packaged #14
Comments
Thank you for reporting this! I tried it out and can reproduce, I know I messed around with this before but remember I'll investigate more and see if this is a regression with newer versions of Chrome. Any ideas for possible solutions would be greatly appreciated! |
This same thing happens in Firefox. It throws a more instructive exception though: |
Would it work to change it to use If I do that, it doesn't throw errors: var a = Object.assign({}, window);
a.crypto.getRandomValues(new Uint8Array(16));
> Uint8Array(16) [154, 140, 83, 149, 168, 44, 48, 218, 5, 237, 230, 179, 224, 10, 238, 173] As an update: this doesn't work. I get problems with constructors not working if I do this. So close! |
This global object patching makes many properties on that object unuseable: Reproduction: (function(global){
// make sure to keep this as 'var'
// we don't want block scoping
var self = Object.create(global);
// ...
// wrapper above
console.log(self.location) // Throws an error described below
})(self); Chrome 72: |
If you want, you can edit the source code to just be That's what I'm doing for some other stuff. |
Yes, but for me, it's a dependency and fiddling around in node_modules isn't particularly persistent 😄 |
Yeah, it's annoying. You could check out a copy of dart-sass, modify it, and then point your package.json to it. |
Hello, sorry for the late response. I know that web workers handle the 'self' keyword differently, could you please file a separate issue for that? Also a separate issue for Firefox support please? I'm not sure if this is something that is an easy fix. Also based on the issues you've reported on other repositories, I would check to see if Dart Sass supports working in the browser. Other factors than just node_preamble may be cause for issues, and are out of the scope of this project. Still searching for a solution, I may just introduce a fix specifically for |
It seems at least Chrome is doing something really weird with the Window object when it's a prototype, try this...
|
I can do that, but these are actually all caused by the same problem: duplicating the
I've got it working, though I have a feeling the dart-sass developers don't really care.
Yes, it's a one-off workaround.
But not for all properties:
|
Great, thank you! I'm aware that duplicating the object isn't perfect, but I feel there are some slight differences to the problem that I discuss below.
Correct, I was speaking directly to the crypto issue described in this ticket. The way I'm looking to tackle it, making the
Any PR could tackle one or more of these problems, but they're also separated to where I can investigate each individually. In addition, it's super important to make sure any change would still fully work with Node.js, as that's the primary goal of this library. |
Is there any traction on this issue? It seems to be effecting more dependencies of this library when using the output in Chrome (perhaps it is behaving more strictly than in the past with code acting on the window object?) |
What about using _globalThis = (typeof globalThis !== 'undefined' && globalThis) ||
(typeof global !== 'undefined' && global) ||
(typeof window !== 'undefined' && window) ||
null;
self = new Proxy(Object.create(_globalThis), {
get(target, prop, receiver) {
try {
return target[prop];
} catch (e) {
if (-1 !== e.message.indexOf('Illegal invocation'))
return _globalThis[prop];
throw e;
}
}
}); |
Because of how the
self
variable is created, any access toself.crypto
throws anUncaught TypeError: Illegal invocation
exception.This will happen when the Random.secure() constructor is called. It compiles to Javascript that looks like this:
But accessing
crypto
will throw.You can try this yourself in Chrome:
var a = Object.create(window);
a.crypto
According to this: uuidjs/uuid#256, it's because it's losing the Crypto context.
My hacky fix is to just make the first line be
var self = global;
and call it good.The text was updated successfully, but these errors were encountered: