regenerator icon indicating copy to clipboard operation
regenerator copied to clipboard

feat: make it strict mode & CSP compatible if possible

Open Jack-Works opened this issue 4 years ago • 16 comments

Use well-known global names if possible

Jack-Works avatar May 28 '20 14:05 Jack-Works

@benjamn hi, can you review this? this is a small PR

Jack-Works avatar Jul 24 '20 14:07 Jack-Works

@benjamn hi, can you review this? this is a small PR

Jack-Works avatar Oct 03 '20 15:10 Jack-Works

@Jack-Works What do you think about this alternate approach? https://github.com/facebook/regenerator/issues/378#issuecomment-764878050

benjamn avatar Jan 21 '21 19:01 benjamn

@Jack-Works What do you think about this alternate approach? #378 (comment)

@benjamn hi that's a cool trick but in some environment like moddable (a JS engine for embedded env) the Object.prototype is frozen. You cannot add things to it. Please at least leave the globalThis try.

Jack-Works avatar Jan 22 '21 02:01 Jack-Works

I'd recommend using all of globalThis, self, and global before resorting to a monkey patch on Object.prototype.

I have a sneaking suspicion that it will cause a massive deoptimization is everything but the newest browsers. The delete is going to turn the Object.prototype into map-mode, which will slow down every single object access. All of them, everywhere, and everything inherits from Object.prototype.

jridgewell avatar Jan 22 '21 04:01 jridgewell

I'm proposing it as the most stable and tested way one more time: https://github.com/zloirock/core-js/blob/v3.8.3/packages/core-js/internals/global.js

zloirock avatar Jan 22 '21 05:01 zloirock

Has this PR stalled? Would love to see it go in for those of us tring to build things in CSP environments.

mikecann avatar Mar 19 '21 06:03 mikecann

I'm just waiting for the maintainer to respond. There aren't so many things I need to up-to-date with.

Jack-Works avatar Mar 19 '21 07:03 Jack-Works

@Jack-Works This PR will fail Content Security Policy, please check the better way that @zloirock has linked in previous comment.

edwardxia avatar May 08 '21 02:05 edwardxia

@Jack-Works This PR will fail Content Security Policy, please check the better way that @zloirock has linked in previous comment.

The original version also violates CSP. I'm making it better on modern browser to use globalThis directly so it don't have to violate CSP

Jack-Works avatar May 08 '21 03:05 Jack-Works

I have rebased to master to resolve merge conflicts.

Jack-Works avatar May 08 '21 03:05 Jack-Works

@Jack-Works This PR will fail Content Security Policy, please check the better way that @zloirock has linked in previous comment.

Updated

Jack-Works avatar May 08 '21 03:05 Jack-Works

Is this issue still relevant? Or is it fixed in regenerator-runtime v.0.13.9? Thank you

juergba avatar Jul 29 '21 07:07 juergba

It's fixed only for engines with globalThis.

zloirock avatar Jul 29 '21 07:07 zloirock

I keep snoozing this tab "Until next month", hoping every time it re-appears that the PR will have been merged. This really seems like a good fix for the described issue, in addition to being backwards-compatible.

On a side note, I can't remember ever actually have gotten very pissed off when debugging, but when I was debugging the strange CSP issue I had almost a year ago, and finally finding out the reason things were failing, the snarky comment that this PR thankfully removes - certainly had me swearing.

Anyhow, I'm crossing my fingers this will be fixed in January! :)

poacher2k avatar Dec 10 '21 14:12 poacher2k

image

I tried as many names as possible. Not only globalThis now.

Jack-Works avatar Dec 11 '21 02:12 Jack-Works