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

Refactor Passthroughs to use subclasses #173

Open
4 tasks
Tracked by #172
trek opened this issue Oct 4, 2016 · 2 comments
Open
4 tasks
Tracked by #172

Refactor Passthroughs to use subclasses #173

trek opened this issue Oct 4, 2016 · 2 comments

Comments

@trek
Copy link
Member

trek commented Oct 4, 2016

Part of #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:

  • Verify that createPassthrough can be defined outside the interceptor function (i.e. does not need to be scoped there).
  • If it must be located inside interceptor document why for future collaborators
  • Convert createPassthrough into more of a class constructor style that "inherits" (in an ES5 sense from FakeXMLHttpRequest) rather than the currently implementation of FakeRequest being an uber class that covers both behaviors
  • Extract this library into its own dependency and update Pretender to import it.
@iezer
Copy link

iezer commented Oct 14, 2016

@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
Copy link
Member Author

trek commented Oct 14, 2016

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

  • it gets extracted
  • an appropriate instance is passed to Pretender
  • the interface to XMLHttpRequest, FakeXMLHttpRequest, and this extracted class are similar enough that we don't need to check "hey, which kind of request is this?" all over the place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants