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

api: fallback to anonymous function #38

Closed
wants to merge 1 commit into from
Closed

api: fallback to anonymous function #38

wants to merge 1 commit into from

Conversation

alex7kom
Copy link
Contributor

vm-browserify uses eval() which is disabled in Chrome Apps (completely)
and Extensions (by default CSP). If can't execute vm.runInThisContext
to create named function, fallback to unnamed function expression.

@indutny
Copy link
Owner

indutny commented Apr 18, 2015

Cool! Now the second step is to detect whether we are running in environment without eval/vm.run... and skip this named thing only for it ;) We still want to have beautiful stack traces in io.js/browser.

@alex7kom alex7kom changed the title api: remove _createNamed method api: fallback to anonymous function Apr 18, 2015
@alex7kom
Copy link
Contributor Author

How about this?

@indutny
Copy link
Owner

indutny commented Apr 18, 2015

Yay! Exactly!

'})');
var named;
try {
named = require('vm').runInThisContext('(function ' + this.name + '(entity) {\n' +
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask you to preserve the 80 column limit that I had here? ;)

@indutny
Copy link
Owner

indutny commented Apr 18, 2015

Just one minor nit, otherwise LGTM! Thank you!

@indutny
Copy link
Owner

indutny commented Apr 18, 2015

(Assuming it works for you ;) )

`vm-browserify` uses `eval()` which is disabled in Chrome Apps (completely)
and Extensions (by default CSP). If can't execute `vm.runInThisContext`
to create named function, fallback to unnamed function expression.
@alex7kom
Copy link
Contributor Author

Done! Yes, it does work for me, with just an uh-oh scary warning in the Chrome console:

developer tools - chrome-extension lmcnjohpfdefcgjggmakchoidldnedfo _generated_background_page html 2015-04-19 11-23-47

@indutny
Copy link
Owner

indutny commented Apr 19, 2015

Thank you! Landed and released!

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

Successfully merging this pull request may close these issues.

2 participants