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

Feature request: A way to mock module, require #5290

Closed
qfox opened this issue Feb 17, 2016 · 12 comments
Closed

Feature request: A way to mock module, require #5290

qfox opened this issue Feb 17, 2016 · 12 comments
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.

Comments

@qfox
Copy link

qfox commented Feb 17, 2016

There is a commit in io.js@2.2.0 that modified behaviour of require: #1801

I understand that this is a major release but some of very handy modules doesn't work now.

For me it looks like we need some legal way (public API) for mocking module, require, and so on.

I guess I can make a PR if you point me.

Related #4190

@MylesBorins MylesBorins added module Issues and PRs related to the module subsystem. feature request Issues that request new features to be added to Node.js. labels Feb 17, 2016
@bnoordhuis
Copy link
Member

Why don't you copy lib/module.js into your own project and patch it up so that it can run outside of core? I think it's highly unlikely we'd commit to an official monkey-patch API, particularly when it's something that can be solved through a third-party module.

@qfox
Copy link
Author

qfox commented Feb 17, 2016

The only reason why I don't copied lib/module: I'm scared of outdating and inconsistency.

We have methods like Module._load, Module._resolveFilename, but there is no method for stat.

I don't see right now how to do it via third party module except copying almost the whole module and fixing right there ;-(.

Here's a cute example: https://github.com/KoryNunn/rooty/blob/master/index.js

@bnoordhuis
Copy link
Member

The public API and the algorithm are fixed. If they ever need to change, it's going to be a semver-major change at the very least so you'll have plenty of time and advance warning.

@qfox
Copy link
Author

qfox commented Feb 17, 2016

Sounds reasonable...

And I've just found another comment about monkey patching. #1801 (comment)

And I'm agree that we shouldn't make useless code.

But lib/module consists of code that could be better unique. If we'll copy it each time we need a new module, we'll get modules like https://github.com/shz/node-require-internal/blob/master/lib/require.js and https://github.com/boblauer/mock-require/blob/master/index.js

This feature request is more like about an ecosystem for mocking module than just monkey patching lib/module in core. Because this situation where we should monkey patch lib/module from user-space code every time without any guarantees (because we patching private methods) is totally crazy.

At least would be great to have some recommendations on mocking core functionality on purpose.

@blond
Copy link

blond commented Feb 17, 2016

👍

1 similar comment
@tadatuta
Copy link

👍

@MauriceButler
Copy link
Contributor

Changing the behavior of require is not terribly difficult in userland I have done it a few times (Im am the author of the above mentioned rooty && rootify )

If it is mocking or changing of apis at runtime you need something like these might help.

fraudster <- Overrides require (Disclaimer another one of my modules)

proxyquire <- Has some good functionality but provides another method to use instead of require

If you are looking for something else that one of these modules dont solve let me know I am happy to help.

@dead-claudia
Copy link

There's also mockery.

@sipayRT
Copy link

sipayRT commented Feb 18, 2016

👍

@vkurchatkin
Copy link
Contributor

You don't need monkey-patching for this. Node provides module loader for CommonJS format, you can provide your own with all the functionality you want. It's much safer than patching Module prototype, Module is not supposed to be used at all.

@vkurchatkin
Copy link
Contributor

To summarise it all:

The only reason why I don't copied lib/module: I'm scared of outdating and inconsistency.

Don't be: algorithm is very stable and fixed. You might not get some performance enhancements, but that's not what you are after anyway.

We have methods like Module._load, Module._resolveFilename, but there is no method for stat.

But you shouldn't use them or patch them. While module system is locked, implementation details can change and these methods can stop doing what you think they do or be removed entirely. Copying the code is just fine.

I don't see right now how to do it via third party module except copying almost the whole module and fixing right there ;-(.

That's exactly how you'd do it, or you can rewrite it from scratch, if you wish. It shouldn't be that hard and you can reuse existing tests to verify compatibility.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 22, 2016

Closing as this seems very unlikely to make it into core.

@cjihrig cjihrig closed this as completed Feb 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

10 participants