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

[React Native] Worker is not defined on tvOS #35

Closed
ghost opened this issue Jun 8, 2017 · 14 comments
Closed

[React Native] Worker is not defined on tvOS #35

ghost opened this issue Jun 8, 2017 · 14 comments

Comments

@ghost
Copy link

ghost commented Jun 8, 2017

Hello !

It is impossible to initialize my pool of threads

This is my code:

let startOptions = {
  maxThreads: 2,
  cache: false,
  debug: true,
  persistence: false,
};

hamsters.init(startOptions);
let params = {
  array: [1,2,3,4,5,6,7,8,9,10],
  animal: 'Hamster',
};
hamsters.run(params, function() {
  if(params.animal === 'Hamster') {
    rtn.data.push('Hamsters are awesome');
  }
}, function(result) {
  console.log(result);
});

What version of the library are you using?

4.1.3

Which platform are you running the library on?

tvOS

Error Message:

ExceptionsManager.js:63 Worker is not defined
handleException @ ExceptionsManager.js:63
handleError @ InitializeCore.js:125
reportFatalError @ error-guard.js:44
__guard @ MessageQueue.js:220
callFunctionReturnFlushedQueue @ MessageQueue.js:100
(anonymous) @ debuggerWorker.js:71

@austinksmith
Copy link
Owner

Please use the provided issue template

@ghost
Copy link
Author

ghost commented Jun 8, 2017

I just have that:

ExceptionsManager.js:63 Worker is not defined
handleException @ ExceptionsManager.js:63
handleError @ InitializeCore.js:125
reportFatalError @ error-guard.js:44
__guard @ MessageQueue.js:220
callFunctionReturnFlushedQueue @ MessageQueue.js:100
(anonymous) @ debuggerWorker.js:71

@austinksmith
Copy link
Owner

austinksmith commented Jun 8, 2017

So it looks like ReactNative no longer includes any Worker support out of the box so you will need to make use of a third party worker implementation within ReactNative moving forward if you want real threading support. I have solved the bug you are receiving and confirmed the library is working in legacy mode at the moment, let me know if you encounter issues with a specific third party implementation.

Do me a favor and give this release a try and report back on your findings, thanks! I've also updated the read me for the project regarding ReactNative support, let me know if you encounter any other issues.

https://github.com/austinksmith/Hamsters.js/releases/tag/v4.1.4

@ghost
Copy link
Author

ghost commented Jun 9, 2017

With 4.1.4 I have this error:

ExceptionsManager.js:71 Unhandled JS Exception: Cannot assign to read only property 'self' of object '#'
reactConsoleErrorHandler @ ExceptionsManager.js:71
console.error @ YellowBox.js:62
logToConsole @ RCTLog.js:45
logIfNoNativeHook @ RCTLog.js:31
__callFunction @ MessageQueue.js:250
(anonymous) @ MessageQueue.js:101
__guard @ MessageQueue.js:218
callFunctionReturnFlushedQueue @ MessageQueue.js:100
(anonymous) @ debuggerWorker.js:71

@austinksmith
Copy link
Owner

austinksmith commented Jun 9, 2017

So it looks like this is a security issue with something Apple has implemented within tvOS or IOS, I do not have any IOS devices on hand nor do I have any macOS devices on hand as I am currently using Ubuntu 16.04 64-bit as my development environment.

If you have any macOS / tvOS / iOS devices that you are willing to donate that will get this sorted significantly quicker as I currently cannot allocate any $ towards purchasing new devices for testing. Or if you or people you know are willing to donate towards the project that will make acquiring these devices much easier.

https://www.patreon.com/asmithdev

I should also mention that anyone can donate towards getting this bug fixed faster using bountysource

https://www.bountysource.com/teams/hamsters.js

@austinksmith
Copy link
Owner

austinksmith commented Jun 9, 2017

The way I had tested this locally was using the latest React Native version and tools while using Android 7.0 and I was getting flawless functionality so this is definitely something unique to Apple devices. Most likely the problem code is

   if(hamsters.wheel.env.node || hamsters.wheel.env.reactNative) {
      global.self = global;
      if(typeof hamsters.Worker !== 'undefined') {
        global.Worker = hamsters.Worker;
      }
    }

Within Node and ReactNative traditionally there is no window object like you would have in the browser, inside threads window is actually referred to as self. As self is the most universally implemented global name space reference across most environments we need to have a global reference to ensure that self actually equates to global for ReactNative to work properly, in Android there is no security restriction preventing you from assigning variables to the global namespace where on iOS and evidently tvOS there is a security restriction preventing that. Hopefully that clarifies the issue for you, my hands are just a little tied at the moment because I'm unable to test any changes made right now on iOS or tvOS devices.

If you edit your installed copy of hamsters on line 101 and change the problem code to this, it might? resolve your issues.

    if(hamsters.wheel.env.node || hamsters.wheel.env.reactNative) {
      global.self = (global.self || global);
      if(typeof hamsters.Worker !== 'undefined') {
        global.Worker = (global.Worker || hamsters.Worker);
      }
    }

@austinksmith austinksmith changed the title React Native: Worker is not defined with v4.1.3 React Native: Worker is not defined on tvOS Jun 9, 2017
@Hizoul
Copy link

Hizoul commented Jun 15, 2017

Did you use the code in a real React-Native Application? If so was it > v0.40?
Because the only Worker Implementation I found for React-Native (react-native-workers) is incompatbile since v0.40 (current is v0.45), and won't let you compile / run your Application.
This means no Webworkers for rn atm, which makes the "React Native Support" advertisted on hamsters.io misleading, since there won't be any performance benefit (or even loss through object cloning) on the Platform until someone updates react-native-workers. I do realize this is very much out of the Scope of hamsters.js itself since it's a WebWorker API wrapper.
I might fork react-native-workers soon and see if I can fix it, if so I'll keep you posted.

// Edit: I found https://github.com/fabriciovergal/react-native-workers
But this needs a seperate RN-JS-Bundler Instance compared to https://github.com/devfd/react-native-workers which doesn't compile with recent versions. Will report back on compatability

@austinksmith
Copy link
Owner

Unfortunately React Native seems to be changing pretty rapidly, when I wrote the support for react native originally about a year ago everything was working fine, now they've completely changed stuff to the point that Android no longer comes with built in worker support (probably to make the platform consistent with IOS), I can make a fork of react native workers and add you as a contributor @Hizoul if you want to work together and fix the issues.

@austinksmith
Copy link
Owner

austinksmith commented Jun 15, 2017

I did not test any specific third party implementation on my end as previously one was not needed for android support, I am unable to test anything related to IOS right now. I did add support for use of third party implementations which is a 1:1 copy of the same support offered which is confirmed working within Node.js

@Hizoul
Copy link

Hizoul commented Jun 16, 2017

Yeah react-native deliberatly stays below v1 because of so many breaking changes and probably more to come :'D
And like I said I do understand very well that this is far out of hamsters.js scope, since this is rn-specific and with webworkers this would work (and did work).
I will be happy to collaborate, I'll keep you posted as soon as I fork and start working on it, but it might be a while before I get time to do it.

@austinksmith
Copy link
Owner

@benbenz You will need to respond to issue tickets you create in the future, tickets that don't have a response from the original writer for more than two weeks will be closed.

@austinksmith
Copy link
Owner

@Hizoul Thanks for the assistance, I'm trying to finish the decoupling effort of the library and once that's complete i'll be able to dedicate some time to fixing the third party implementation out there. I find it frustrating that this is arbitrarily limited now since at least Android had full support for workers previously.

@austinksmith
Copy link
Owner

@benbenz Can you please test this again using v4.2.0?

@austinksmith austinksmith changed the title React Native: Worker is not defined on tvOS [React Native] Worker is not defined on tvOS Oct 2, 2017
@austinksmith
Copy link
Owner

@benbenz I am closing this ticket due to inactivity, there is an open issue currently related to React Native blob support (lack there of), in the mean time please use v4.2.1 as it should solve the "self" issues this ticket was referencing and you will have the library in place for when RN fixes their blob support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants