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

Only remove non-native modules from cache #149

Merged
merged 2 commits into from
Jan 26, 2017
Merged

Only remove non-native modules from cache #149

merged 2 commits into from
Jan 26, 2017

Conversation

sonicdoe
Copy link
Contributor

@sonicdoe sonicdoe commented Jan 4, 2017

As discussed in #109 and #108, we should only remove non-native modules from the cache because, upon requiring a native module a second time, it causes a “Module did not self-register” error (see nodejs/node#5016).

I thought about adding a similar check to _withoutCache when noPreserveCache() is used but I’m not sure if that’s desirable because it conveys the idea that native modules can be cleared from cache. Maybe we should add a note explaining that noPreserveCache() shouldn’t be used when stubbing native modules?

sonicdoe and others added 2 commits January 4, 2017 21:23
Removing a native module from the cache and requiring it a second time causes a “Module did not self-register” error.

See nodejs/node#5016.

Co-Authored-By: Hubert SABLONNIÈRE <hubert.sablonniere@gmail.com>
@bendrucker
Copy link
Collaborator

Please take a look at the CI failures

@sonicdoe
Copy link
Contributor Author

sonicdoe commented Jan 4, 2017

I have fixed the Travis CI build by installing a compiler as explained in the documentation.

@bendrucker bendrucker merged commit 6df0a6a into thlorenz:master Jan 26, 2017
@bendrucker
Copy link
Collaborator

Sorry for the delay here, appreciate the contribution

@sonicdoe sonicdoe deleted the native-modules branch January 26, 2017 19:28
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.

2 participants