Make node core modules work on modulePathIgnorePatterns #122

adaschevici · 2014-08-29T14:45:02Z

...ock #106
This is not the best fix but it solves the problem for node modules.
I would appreciate some feedback on ways to improve it.
Need to write the tests still.

…d mock #106

facebook-github-bot · 2014-08-29T14:46:37Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

facebook-github-bot · 2014-08-29T14:58:38Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

ovidiuch · 2014-09-02T12:58:17Z

@adaschevici

The title seems to the truncated message of the first commit, can you add a more suggestive title like "Make modulePathIgnorePatterns work for node modules."?
You mention you didn't write tests, but did you run existing ones to see if they all pass?

ovidiuch · 2014-09-02T12:59:04Z

src/HasteModuleLoader/HasteModuleLoader.js

          return this._configShouldMockModuleNames[moduleName] = false;
        }
+


You leaked a new line here, remove it with the next commit.

adaschevici · 2014-09-02T13:05:45Z

@skidding the existing tests pass.

jeffmo · 2014-09-04T16:32:41Z

src/HasteModuleLoader/HasteModuleLoader.js

@@ -123,7 +123,6 @@ function Loader(config, environment, resourceMap) {
  this._reverseDependencyMap = null;
  this._shouldAutoMock = true;
  this._configShouldMockModuleNames = {};
-


Can we leave little stuff like this as-is so that git blame signal remains useful?

@jeffmo addressed spacing.

duereg · 2014-09-19T19:54:19Z

any progress on this?

martindanielson · 2014-10-02T14:32:41Z

I included this fix and got this instead:

c:\node_modules\d3\index.js: Cannot read property 'id' of undefined
    at Object.exports.jsdom (c:\node_modules\d3\node_modules\jsdom\lib\jsdom.js:51:33)
    at c:\node_modules\d3\index.js:1:100
    at Object.runContentWithLocalBindings (c:\node_modules\jest-cli\src\lib\utils.js:315:17)

Running on Windows with the following versions:

    "d3": "^3.4.11",
    "jest-cli": "^0.1.17",

Any thoughts on that or someone else with the same issue?

jeffkole · 2015-01-20T18:16:28Z

I applied this fix to master (v0.2.1) and still get the same bug, though the fix works just fine against v0.1.18. Are there any updates on this PR and the issue it addresses?

adaschevici · 2015-01-20T18:58:37Z

@jeffkole hey....this was forever ago. I need to figure out what I was doing then and how i would fix it now.
Will probably get to it sometime this week.

Cheers.

jeffkole · 2015-01-20T20:48:37Z

Thank you

jeffmo · 2015-01-23T02:40:45Z

src/HasteModuleLoader/HasteModuleLoader.js

@@ -591,7 +591,7 @@ Loader.prototype._shouldMock = function(currPath, moduleName) {
      this._configShouldMockModuleNames[moduleName] = true;
      for (var i = 0; i < this._unmockListRegExps.length; i++) {
        unmockRegExp = this._unmockListRegExps[i];
-        if (unmockRegExp.test(modulePath)) {
+        if (unmockRegExp.test(modulePath) || modulePath === null) {


Won't this just never mock node built-ins?

Instead, shouldn't this be something like
unmockRegExp.test(modulePath) || (modulePath === null && unmockRegExp.test(moduleName)?

Otherwise I think this will just always mark node built-ins as unmocked, right?

Yes...this makes the core modules no longer be ignored.

ghost · 2015-08-05T18:01:07Z

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

danielstern · 2015-09-09T12:46:30Z

This would be a very good fix, as this is a bug that can be a bit frustrating very early in the implementation process.

pheuter · 2015-10-12T20:05:35Z

+1

cpojer · 2015-10-14T23:28:18Z

@adaschevici are you still interested in providing a proper fix for this issue?

adaschevici · 2015-10-15T20:14:17Z

@cpojer looking into it as the code has shifted around quite a bit since I submitted this pull request.

adaschevici · 2015-10-18T23:14:56Z

Hi guys,

Not sure but this has evolved over time and as far as I can tell the modulePathIgnorePatterns is no longer used and the initlal problem was fixed, and i believe the new requirements should be something like:

Make the modulePathIgnorePatterns use aliases in the node_modules. (for example in jquery)
eg add 'jquery' to modulePathIgnorePatterns would make jest not automock the lib.
Am I going into the right direction?

github-actions · 2021-05-14T21:06:30Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

Added a double check for Node modules and extended the check in shoul…

db4bb08

…d mock #106

Remove debug statements #106

a03e1ae

Refactor condition to be more concise #106

eb67eea

ovidiuch reviewed Sep 2, 2014
View reviewed changes

adaschevici changed the title ~~Added a double check for Node modules and extended the check in should m...~~ Make node core modules work on modulePathIgnorePatterns Sep 2, 2014

Remove redundant parantheses #106

4fac716

jeffmo reviewed Sep 4, 2014
View reviewed changes

Readd spaces to file to preserve style #106

cf4c6ff

This was referenced Sep 8, 2014

cannot run tests that require d3.js #83

Closed

jest.dontMock('debug') not respected #77

Closed

jeffmo reviewed Jan 23, 2015
View reviewed changes

facebook-github-bot added the CLA Signed ✔️ label Apr 7, 2015

cpojer closed this Oct 23, 2015

github-actions bot locked as resolved and limited conversation to collaborators May 14, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make node core modules work on modulePathIgnorePatterns #122

Make node core modules work on modulePathIgnorePatterns #122

adaschevici commented Aug 29, 2014

facebook-github-bot commented Aug 29, 2014

facebook-github-bot commented Aug 29, 2014

ovidiuch commented Sep 2, 2014

ovidiuch Sep 2, 2014

adaschevici commented Sep 2, 2014

jeffmo Sep 4, 2014

adaschevici Sep 5, 2014

duereg commented Sep 19, 2014

martindanielson commented Oct 2, 2014

jeffkole commented Jan 20, 2015

adaschevici commented Jan 20, 2015

jeffkole commented Jan 20, 2015

jeffmo Jan 23, 2015

adaschevici Oct 18, 2015

ghost commented Aug 5, 2015

danielstern commented Sep 9, 2015

pheuter commented Oct 12, 2015

cpojer commented Oct 14, 2015

adaschevici commented Oct 15, 2015

adaschevici commented Oct 18, 2015

github-actions bot commented May 14, 2021

		return this._configShouldMockModuleNames[moduleName] = false;
		}

Make node core modules work on modulePathIgnorePatterns #122

Make node core modules work on modulePathIgnorePatterns #122

Conversation

adaschevici commented Aug 29, 2014

facebook-github-bot commented Aug 29, 2014

facebook-github-bot commented Aug 29, 2014

ovidiuch commented Sep 2, 2014

ovidiuch Sep 2, 2014

Choose a reason for hiding this comment

adaschevici commented Sep 2, 2014

jeffmo Sep 4, 2014

Choose a reason for hiding this comment

adaschevici Sep 5, 2014

Choose a reason for hiding this comment

duereg commented Sep 19, 2014

martindanielson commented Oct 2, 2014

jeffkole commented Jan 20, 2015

adaschevici commented Jan 20, 2015

jeffkole commented Jan 20, 2015

jeffmo Jan 23, 2015

Choose a reason for hiding this comment

adaschevici Oct 18, 2015

Choose a reason for hiding this comment

ghost commented Aug 5, 2015

danielstern commented Sep 9, 2015

pheuter commented Oct 12, 2015

cpojer commented Oct 14, 2015

adaschevici commented Oct 15, 2015

adaschevici commented Oct 18, 2015

github-actions bot commented May 14, 2021