Rewrite require('proxyquire') calls

[bendrucker]

I wrote proxyquire-universal to allow you to use proxyquire in your tests and then turn it proxyquireify via a browserify plugin. From Thorsten via email:

BTW: this (https://github.com/bendrucker/proxyquire-universal/blob/master/src/transform.js#L12) is pretty brittle as it would match inside messages, etc. as well. Also it’d miss require(“proxyquire”) (double quotes).

Seems like it’d be better integrated with proxyquireify to avoid overhead and order dependance of an extra transform, but lets keep discussing there.

Wrote it up quickly to solve @beckyconning's issue in https://github.com/Nikku/karma-bro/issues/40 so it's definitely a little brittle.

I haven't done a lot of work with syntax parsers — any recs on what to use there?

[bendrucker]

Just pushed 1.0.1 written using jstransform instead of simple string replacement (see https://github.com/bendrucker/proxyquire-universal/commit/037e056fd2c97677d8301ca11271666175df6269)

[thlorenz]

That's nice, actually have a look at detective and how it is used in exposify to replace require('x') with (window.x) statements.

Using that you don't have to write this all by hand and get to reuse (and possibly improve) an existing lib for that. It's a super solid lib for finding specific function calls (I actually fixed some problems in the process). It will definitely cover many more edge cases than you probably thought of.

Also notice how exposify checks if a require is even present before performing the much more expensive AST parse.

I'm open to a PR to exposify to make the replacement function configurable so you could use that for your task instead of basically having to copy/paste this code

WRT making it part of proxyquireify I'm not sure anymore. We'd parse the AST twice either way, so speed wouldn't be improved. However people have to know to apply your plugin before proxyquireify. How can they reliably specify that? In either case you should make that very clear in your readme if we keep things separate.

Including it into proxyquireify would work around that issue since we could control the order of things. So I'll leave this decision up to you, but would be happy to merge a tested PR that adds this feature directly to proxyquireify.

[bendrucker]

What I'm actually doing is applying proxyquireify as a plugin from within my plugin. I'd be happy to get this added to exposify and proxyquireify instead of having something additional for users to think about.

[bendrucker]

Wanted to loop back on this. I'm all for reusing code from exposify, but wouldn't it make more sense to extract that functionality into a separate module and have both depend on it?

[beckyconning]

:+1:

[thlorenz]

Yeah, I'm still not sure if this belongs anywhere in the packages you mentioned.

• exposify definitely has nothing to do with either proxyquire and should not need to know anything about them
• proxyquireify could know about proxyquire, but I think it's problematic

I think the best idea is actually to have a unifying package that knows about both and adapts the code to automatically use the right one depending on the situation. proxyquire-universal could be that package. We could add links to it on the readme of both proxyquire and proxyquireify to make it easily discoverable.

wouldn't it make more sense to extract that functionality into a separate module and have both depend on it?

if we go the route I suggested, only proxyquire-universal would have to depend on it

instead of having something additional for users to think about.

Not much for the user to think about since there are 3 simple cases:

• a) your tests only run server side -> use proxyquire
• b) your tests only run client side -> use proxyquireify
• c) your tests run server side and client side -> use proxyquire-universal

[bendrucker]

I'm with you. In that case, the only remaining todos are documentation updates both here and on proxyquire, correct? I'll extract the require replacement transform into its own package when I have a chance but that's an internal implementation detail for now. Will send PRs for both proxyquire* repos if we're on the same page.

[thlorenz]

Thanks, sounds good. Possibly you should also add the three cases I outlined to the proxyquire-universal page so users have it easy to decide when to use either proxyquire solution.

[bendrucker]

Cool. README commits on all three it is. Will send PRs this evening.

[thlorenz]

Sounds good, one thing I'd suggest is directly depend on proxyquire and proxyquireify.

The reason is that you'd expect the user to run tests on client and server and thus they need both modules anyways.

The only reason not to do so that I can think of is to allow users to pick the version for each proxyquire.

[bendrucker]

Yeah that makes sense. I don't see any reason to allow people to pick their versions versus just matching major versions on both and then keeping up to date in the rare cases of major bumps.

[thlorenz]

Added you as collab here as well, so just update the README and let me review the PR then push to master. I'll republish. Adding you to proxyquire as well so we can successfully manage the proxyquire* repos together and make them easy to use.

One other thing that'd be nice is if proxyquire-universal would document some API differences that do exist. For example proxyquireify cannot do any global overrides like proxyquire.