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

3.12.0 breaks re-requires of native modules #178

Closed
rosston opened this issue Oct 31, 2016 · 2 comments
Closed

3.12.0 breaks re-requires of native modules #178

rosston opened this issue Oct 31, 2016 · 2 comments

Comments

@rosston
Copy link

rosston commented Oct 31, 2016

The require cache clearing in #141 breaks re-requires of native modules (see nodejs/node#6160). So if I have one test file that requires a native module, another test file that requires mock-fs, and a 3rd test file that requires the same native module; the final test file will crash because it cannot require the native module.

I have an (extremely-simplified) example here: https://github.com/appropos/mock-fs-native-module-bug

I originally planned to provide a failing test and working fix in a PR, but it's difficult to test. If I add an integration test that more or less does,

require('iconv');
require('../../lib/index');
require('iconv');

the test passes because the ../../lib/index require is cached, meaning the cache-clearing doesn't happen again. I tried explicitly clearing ../../lib/index from the require cache in my test, but that broke all other tests.

Tests or not, I imagine the fix is as simple as not clearing require.cache entries that end in .node.

EDIT: See #180 for a test and fix.

rosston added a commit to appropos/mock-fs that referenced this issue Nov 1, 2016
rosston added a commit to appropos/mock-fs that referenced this issue Nov 1, 2016
@tschaub
Copy link
Owner

tschaub commented Nov 1, 2016

Thanks for the report and the proposed fix in #178. I'm inclined to revert #141 since that appears to have introduced other problems. I'll issue a patch release and then revisit the cache clearing issue (it may turn out to be the wrong solution altogether).

@rosston
Copy link
Author

rosston commented Nov 1, 2016

Thanks! #181 / 3.12.1 fix the issue.

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

2 participants