Skip to content

Enable module.exports mocking in react-test.js#156

Closed
benjamn wants to merge 6 commits intofacebook:masterfrom
benjamn:issue-155-mock-modules
Closed

Enable module.exports mocking in react-test.js#156
benjamn wants to merge 6 commits intofacebook:masterfrom
benjamn:issue-155-mock-modules

Conversation

@benjamn
Copy link
Contributor

@benjamn benjamn commented Jul 3, 2013

We don't currently attempt to mock modules automatically, but we do respect require("mock-modules").mock, .dontMock, and .dumpCache.

I'm going to keep investigating auto-mocking, since that would move us much closer to the behavior used within Facebook.

cc @jeffmo @zpao @petehunt @tomocchino @jordwalke

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this appropriate to commit upstream? Auto-mocking would take care of this in theory, but the details need some more work.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fine to do upstream -- it should be equivalent/redundant with automocking.
You might just add a comment so someone knows why you did it if they don't understand the redundancy

Copy link
Contributor

Choose a reason for hiding this comment

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

Mocking ReactDefaultInjection will break React, since it configures a bunch of things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@petehunt are you saying that ReactDefaultInjection should not be auto-mocked upstream by default (as far as I know it is currently mocked automatically)?

Copy link
Contributor

Choose a reason for hiding this comment

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

It is not mocked automatically. Everything in React is exempted by default. If ReactDefaultInjection were mocked React would not know anything about the browser, since it's where we inject the events system, DOM properties and tags.

@benjamn
Copy link
Contributor Author

benjamn commented Jul 8, 2013

This PR depends on #177, just to be clear.

benjamn added 6 commits July 9, 2013 17:03
This is far from ideal, but it seems to be the easiest way to expose the
require.dumpCache method for use by require("mock-modules").dumpCache.

Upstream patch that would make this hack unnecessary if accepted:
browserify/browser-pack#16
We don't currently attempt to mock modules automatically, but we do
respect require("mock-modules").mock, .dontMock, and .dumpCache.

I'm going to keep investigating auto-mocking, since that would move us
much closer to the behavior used within Facebook.

Closes facebook#154.
Closes facebook#155.
The meaning of __DEV__ is pretty overloaded, so depending on that was
asking for confusion.
When require("mock-modules").dumpCache() is called, all mock functions
previously created continue to refer to the old dirtyMocks array.

If we replace that array with a new one, those mock functions will never
have their .mockClear() methods called again.

The upstream version of mocks.js pulls a similar global trick, and I never
understood why until now.
Copy link
Member

Choose a reason for hiding this comment

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

Bad @benjamn. No modifying files unnecessarily.

@benjamn benjamn mentioned this pull request Jul 15, 2013
@benjamn
Copy link
Contributor Author

benjamn commented Jul 17, 2013

Superseded by #193.

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.

4 participants