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

Stop extending types when replacing constructors #254

Merged
merged 2 commits into from Jun 2, 2017
Merged

Conversation

searls
Copy link
Member

@searls searls commented Jun 2, 2017

This is a breaking change

I made a mistake in td 2.0 and it took too long to catch. Mea culpa. I feel bad.

What went wrong

I've decided after fighting various babel shenanigans to give up on trying to create test double constructors that will pass instanceof checks. The reality is that the ES Class spec buttons up any cuteness I'd been able to previously get away with, namely

  1. There's no way to pass an instanceof check of a class without extending it
  2. There's no way to extend a class without invoking the super constructor upon instantiation (the sole alternative is to instantiate it manually and return that from the sub constructor, but for our purposes that has the same downside—production code of a dependency getting executed by a unit test).
  3. We have since learned that when babel-transpiled code attempts to extend a bona fide ES class, all hell breaks loose, and there's no real way to work around it currently. Namely, the babel-transpiled constructor will attempt to call the superclass constructor without new, which violates a runtime check of the ES class spec.

New behavior

Going forward, td.replace() on constructors will not extend the original type, and will instead copy over attribute values and imitate functions. The previous behavior will be hidden behind a global configuration option called td.config({extendWhenReplacingConstructors: false}) for the handful of folks who really want their test doubles to pass instanceof checks (which I suspect is a small minority, given how sparingly the instanceof keyword is typically used).

Resolves #242

The biggest ramification of this is that static & instance attributes
defined for the type will not be copied over. Probably nothing to be
done about most instance attributes (since the whole point of this is to not
call the real constructor, and that's when all non-contrived instance
attrs are set).

Plan is to shallow copy over non-function properties from the type and
the prototype and see where that gets us.
I chose to copy them over in all cases, because if there is a difference
between getting the property via extension vs not, I really don't want
this tdjs option to be the reason someone's test broke
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.

None yet

1 participant