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

TypeError when requiring sinon in React Native 0.32.0 #1143

Closed
simonsmith opened this issue Sep 7, 2016 · 13 comments
Closed

TypeError when requiring sinon in React Native 0.32.0 #1143

simonsmith opened this issue Sep 7, 2016 · 13 comments

Comments

@simonsmith
Copy link

Opened after discussion in #1051 (comment)

  • Sinon version: 2.0.0-pre.2
  • Environment:
    • OSX 10.11.6
    • node 6.3.1
  • Other libraries:
    • React Native 0.32.0
    • Jest 15.0.0

What did you expect to happen?

I expect it it work in Jest when running tests against React Native

What actually happens

It throws an error when trying to require the sinon library:

const sinon = require('sinon');

Throws the following error:

    TypeError: Cannot read property 'now' of undefined

      at setTimeout (node_modules/react-native/Libraries/JavaScriptAppEngine/System/JSTimers/JSTimers.js:50:45)
      at /Users/simonsmith/Sites/new-look/newlookapp/node_modules/lolex/src/lolex-src.js:32:25
      at Object.<anonymous> (node_modules/lolex/src/lolex-src.js:655:2)
      at Object.<anonymous> (node_modules/sinon/lib/sinon/util/fake_timers.js:20:11)

JSTimers.js

How to reproduce

Install react-native, jest and sinon.

Add a unit test like the following:

require('react-native');
const React = require('react');
const renderer = require('react-test-renderer');
const sinon = require('sinon');

describe('Test', () => {
  it('renders correctly', () => {

  });
});

Run jest and see the error.

I realise it can be a hassle to set RN up so if you'd like a repo with the error present let me know and I'll create one.

Thanks

@fatso83
Copy link
Contributor

fatso83 commented Sep 7, 2016

Hi, if I should look into this I don't want to be doing more setup than a git clone foo && npm install && npm test, so a repo would be nice.

@andpor
Copy link
Contributor

andpor commented Sep 14, 2016

@fatso83 Carl from initial glance it looks like the setTimeout from lolex-src from line 32: var timeoutResult = setTimeout(NOOP, 0); somehow is resolving into a RN setTimeout...and barfs on Date.now() ??

@andpor
Copy link
Contributor

andpor commented Sep 14, 2016

@simonsmith can you try with latest RN 0.34-rc0. I see JSTimers.js heavily refactored - maybe the issue is resolved in RN?

@thormartin91
Copy link

I am experiencing the same issue, with RN 0.34.1 as well.
Also, when using ES6 import: import sinon from 'sinon';, I get a slightly different error:

TypeError: Date is not a constructor

  at new MessageQueue (node_modules/react-native/Libraries/Utilities/MessageQueue.js:62:26)
  at Object.<anonymous> (node_modules/react-native/Libraries/BatchedBridge/BatchedBridge.js:17:19)
  at Object.<anonymous> (node_modules/react-native/Libraries/Utilities/RCTLog.js:14:19)
  at setUpConsole (node_modules/react-native/Libraries/JavaScriptAppEngine/Initialization/InitializeJavaScriptAppEngine.js:54:1)

@fatso83
Copy link
Contributor

fatso83 commented Oct 8, 2016

@thormartin91 could you create a repo that makes it possible to reproduce this easily? I am not likely to debug unsupported platforms I am not using just for fun, especially if it's a hassle to reproduce the error :-)

@andpor
Copy link
Contributor

andpor commented Oct 8, 2016

@thormartin91 if you create this simple repo I will take a look at the issue.

@thormartin91
Copy link

thormartin91 commented Oct 8, 2016

This might actually be a problem with mocking of a component, hence a constructor being called. I will continue to investigate and report back :)

@thormartin91
Copy link

I have not managed to solve this issue, so I created a repository to reproduce it: sinon-issue-1143. The exception occurs if you import both sinon and a AppComponent in the same test-file. Importing only one works fine. You'll find an example within the repo, in the file Component.test.js

@andpor
Copy link
Contributor

andpor commented Oct 11, 2016

@thormartin91 - Just FYI, RN is not officially supported by sinon. Here you are talking about JEST + SINON on RN. So it is double not supported. I had not had an issue using sinon by itself with RN so far with the PR that was merged several months ago. I will take a look at your repo to see if there is anything obvious but I would not expect miracle - just to set expectations...

@thormartin91
Copy link

No problem, @andpor.
I intended to use sinon to spy on functions, but have solved it with jest by using:

const mockFunction = jest.fn(() => 'Mock function')
expect(mockFunction).toBeCalled()

@mroderick
Copy link
Member

Based on @thormartin91's test case, I was able to create a much reduced test case that shows the error in effect.

https://github.com/mroderick/sinon-issue-1143

I have created sinonjs/fake-timers#87 to fix the issue.

Once that has been merged and a new version has been published, I think we should publish a new version of Sinon 2.x.

Sinon.JS 1.x is locked to [email protected]. Shall we try upgrading lolex for that branch also and cut a new release?

@mroderick
Copy link
Member

Related: jsdom/jsdom#1631

@mroderick
Copy link
Member

Once that has been merged and a new version has been published, I think we should publish a new version of Sinon 2.x.

It won't be necessary to publish a new 2.x pre-release, as the package.json specifies ^1.4.0

Sinon.JS 1.x is locked to [email protected]. Shall we try upgrading lolex for that branch also and cut a new release?

I'll focus on some more urgent tasks than this. We can re-visit it if there are requests for it, or pull requests that implements it.

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

No branches or pull requests

5 participants