pretenderjs / pretender

A mock server library with a nice routing DSL
MIT License
1.26k stars 158 forks source link

Refactor Passthroughs to use subclasses #173

Open trek opened 8 years ago

trek commented 8 years ago

Part of https://github.com/pretenderjs/pretender/issues/172, which contains lots of explanatory writing to make this PR more approachable by those new to Pretender. Check that Issue out!


We're currently unhappy with the implementation of Passthrough requests (see #172 for more information about Passthrough's currently implementation and use).

Right now we detect whether an incoming request is stubbed or needs to be passed through here. This part is fine but:

  1. the function for creating a passthrough is run on every "interception" event, which seems odd. There may be a use case for this. If so, please update internal documentation for future readers.
  2. createPassthrough seems to be acting like a sub class of FakeXMLHttpRequest. If that's the case, we'd like to clarify the intent and make it easier to extract this class into its own library at a later date.

PRs that successfully help solve this issue will:

iezer commented 8 years ago

@trek I'm taking a stab at this issue. For (1) we can definitely define createPassthrough outside of interceptor as I've done here.

For (2), I'm not sure createPassthrough is really acting as a subclass of FakeXMLHttpRequest. This function makes a real XMLHttpRequest and modifies all the events and properties on the fake instance so that the are hooked up to the real request, for example, if an event fires on the real request, the same event fires on the fake, and if a property changes on the real one, that change is propagated to a fake.

I can see the value of making a Proxy class for #174, but I think that Proxy class can be a subclass of XMLHttpRequest, not FakeXMLHttpRequest, which stores all responses to localStorage.

trek commented 8 years ago

For (2), I'm not sure createPassthrough is really acting as a subclass of FakeXMLHttpRequest.

I'm not terribly concerned with the actual class hierarchy, just that