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

Problem with spies #35

Open
mindplay-dk opened this issue Apr 16, 2021 · 3 comments
Open

Problem with spies #35

mindplay-dk opened this issue Apr 16, 2021 · 3 comments

Comments

@mindplay-dk
Copy link

I'm trying to spy on removeChild in a test - I have a parent node being modified like this:

    let removals = 0;

    let removeChild = parent.removeChild.bind(parent);

    parent.removeChild = (child) => {
      removals += 1;
      return removeChild(child);
    };

To my surprise, it was counting way too many calls to removeChild.

It turns out, several of these methods are being called internally, for brevity.

The real DOM does not call it's own public methods, afaik? If it needs to remove children for other reasons than calls to removeChild, it does not internally call the method, it just removed them.

How would you feel about moving the internally reused methods to private methods?

@mindplay-dk
Copy link
Author

To illustrate what I'm proposing:

image

For now, I'm using a modified copy of this library in my test-suite, and just made this minimal change so I can finish. If you'd like, I'd be happy to go over the rest and submit a PR to make sure no methods are calling any public methods.

(I supposed these methods could be #private as well, but I don't know how you feel about using private class fields with only two thirds of browsers supporting them? Though I suppose microbundle takes care of all that...)

@developit
Copy link
Owner

@mindplay-dk I think we can just add internal helper methods that live off the class, like remove(parent, child).

@mindplay-dk
Copy link
Author

"Off the class", so not methods, just functions? Like this?

function createEnvironment() {
	// ...
	function remove(parent, child) {
		splice(parent.childNodes, child, false, true);
		return child;
	}
	// ...
	class Node {
		// ...
		removeChild(child) {
			return remove(this, child);
		}
		remove() {
			if (this.parentNode) remove(this.parentNode, this);
		}
	}
	// ...
}

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

No branches or pull requests

2 participants