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

Improve support for custom require hooks #65

Merged
merged 1 commit into from
Dec 1, 2015

Conversation

novemberborn
Copy link
Contributor

First of, thanks for adding source map support! I've been trying to use nyc@4 with the project that lead to my code coverage article from last week. I got it working but had to make some changes. This is the first PR which covers the custom require hook implementation.


The custom hook should decide which modules it wants to ignore. For example Babel lets you customize this beyond just node_modules using the only and ignore options.

Custom hooks often store the hook they're replacing, and may call this hook when loading a module (e.g. if they decide to ignore that particular module). Hooks may also be removed, restoring the original hook. Libraries like proxyquire always fall back to the original hook but modify the module object.

The implementation was changed to:

  • always pass the original module object to the custom hook
  • support restoring older hooks
  • correctly compile the code by using Module.prototype._compile

It's getting late in the day in my timezone so I haven't had a chance to update the tests to cover this new behavior. The test suite in my own project passes though when run under nyc so that's encouraging.

Let the custom hook decide which modules it wants to ignore. For example Babel
lets you customize this beyond node_modules.

Custom hooks often store the hook they're replacing, and may call this hook when
loading a module (e.g. if they decide to ignore that particular module). Hooks
may also be removed, restoring the original hook. Libraries like proxyquire
always fall back to the original hook but modify the module object.

The implementation was changed to:

* always pass the original module object to the custom hook
* support restoring older hooks
* correctly compile the code by using Module.prototype._compile
@bcoe
Copy link
Member

bcoe commented Nov 30, 2015

@novemberborn this is looking great, thanks for the contribution! I'll have some comments after the work day today.


module._compile(obj.content, filename)
var wrapCustomHook = function (hook) {
return function (module, filename) {
Copy link
Member

Choose a reason for hiding this comment

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

I love this approach, much cleaner than what I was doing before -- also I think that it fixes the issue I was originally having that required me to ignore the node_modules folder -- great job.

@bcoe
Copy link
Member

bcoe commented Dec 1, 2015

@novemberborn this is great work, and I think we're sufficiently covered by the existing test cases -- I also tested it on several libraries I've instrumented with nyc (yargs, async, pageres, yarsay).

I say :shipit:

bcoe added a commit that referenced this pull request Dec 1, 2015
Improve support for custom require hooks
@bcoe bcoe merged commit ee02509 into istanbuljs:master Dec 1, 2015
@novemberborn novemberborn deleted the improve-require-hook branch December 1, 2015 20:23
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