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

enhancement: support window.eval() in Dev build browser #628

Closed
theWebalyst opened this issue Mar 12, 2019 · 4 comments

Comments

5 participants
@theWebalyst
Copy link

commented Mar 12, 2019

SAFE Browser lacks support for window.eval(), I think because it introduces security risks.

It is though a useful feature when debugging, because more efficient, more developer friendly source maps make use of it, including the default applications built by frameworks such as React.

For example, debugging an app created using create-react-app which supported not reloading with webpack eval-source-map works fine in other browsers but fails to run with SAFE Browser. I was able to fix this, but to do so had to npm run eject in order to change the config to use a different source map ('cheap-module-source-map’) which is an irreversible process, and gives much poorer source level debugging.

So I propose adding support for window.eval() in the Dev build browser, but with a suitable warning in the console the first time it is called, to reduce the chances of someone using it in their producing build.

There's a post about the issue in the Dev forum here

@joshuef joshuef added this to Needs triage in Upcoming release ('master' branch) via automation Mar 15, 2019

@joshuef joshuef moved this from Needs triage to Low priority in Upcoming release ('master' branch) Mar 19, 2019

@joshuef

This comment has been minimized.

Copy link
Collaborator

commented Apr 2, 2019

Aye this seems reasonable.

The ideal would be only for localhost sites, IMO. Just giving us a bit more security in case a dev is using the browser for normal browsing too.

so if mock && localhost should be fine. I don't see any obvious issues with that.

@theWebalyst

This comment has been minimized.

Copy link
Author

commented Apr 2, 2019

Does it make sense for that to be the default, but to have a way to do this with the live network for that nasty once in a while bug that either only shows when live or is just easier to handle that way?

@hunterlester hunterlester self-assigned this Apr 4, 2019

@hunterlester hunterlester added this to the Next milestone Apr 4, 2019

@hunterlester hunterlester moved this from Low priority to In Progress in Upcoming release ('master' branch) Apr 4, 2019

@hunterlester hunterlester moved this from In Progress to Needs review in Upcoming release ('master' branch) Apr 4, 2019

@joshuef joshuef closed this in #653 Apr 5, 2019

Upcoming release ('master' branch) automation moved this from Needs review to Ready For QA Apr 5, 2019

@safesurfer

This comment has been minimized.

Copy link
Contributor

commented Apr 9, 2019

QA verified, I'm able to run eval() and it is allowed in mock, and I can see the error message Sorry, peruse does not support window.eval(). only on production.

@safesurfer safesurfer moved this from Ready For QA to Done in Upcoming release ('master' branch) Apr 9, 2019

@theWebalyst

This comment has been minimized.

Copy link
Author

commented Apr 30, 2019

@joshuef Did this make it into v0.13?

@calumcraig calumcraig added the 0.13.0 label May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.