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

More rigorous checking of optional arguments #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stt
Copy link

@stt stt commented Aug 1, 2015

I was looking to do array filtering in a webworker and noticed that context was not set correctly as it though it was a list of imports.

By the way, although retrospectively obvious, it might be worth noting somewhere clearly that native functions can't be serialized by JSONfn. 😄
(I used the polyfill from https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Array/filter)

I.e. allow context to be an array.
@vkiryukhin
Copy link
Owner

Hi stt,

Good catch!
I have a couple of comments:

try {
    return eval('(' + value + ')');
 } catch(e) {
    console.log(e);
    return value;
  }
  1. would be nice to find more elegant way to implement try{...} part logic. Maybe you have any idea.
  2. console.log() not needed in production of course.

I haven't work on this plugin for a while. If would useful if you provide most simple and most complex examples of object which you use for testing.

Thank you for working on plugin's improvement.

--Vadim

@stt
Copy link
Author

stt commented Aug 4, 2015

This was probably meant as comment to #6 but only option for try-catch I see now is to do the "[native code]" string checking before calling or evaling it in JSONfn, either way it would duplicate couple of checks.
I'm not using the patch for real yet, but was looking to do something like this

var _filtered = ["asd","zxc"];
var fn = function filterBySomething(s) { return s=="asd"; };
vkthread.exec(_filtered.filter, [fn],
      function(data){
        _filtered = data;
      },
      _filtered);

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