Skip to content

Conversation

sheerun
Copy link
Contributor

@sheerun sheerun commented Oct 12, 2018

This PR supersedes #119, incorporates all your changes, checks if window.MutationObserver is available and uses it instead of using mock, and takes care of wait-for-dom-change

return new Promise((resolve, reject) => {
const timer = setTimeout(onTimeout, timeout)
const observer = new window.MutationObserver(onMutation)
const MutationObserverConstructor =
Copy link
Contributor Author

@sheerun sheerun Oct 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left this code because I realised this can be useful if someone loads custom shim of MutationObserver after '@sheerun/mutationobserver-shim' is already loaded

@sheerun
Copy link
Contributor Author

sheerun commented Oct 12, 2018

The failing test is not for my code at all

@kentcdodds
Copy link
Member

Ok, I've made it so the tests run via projects so they run at the same time. I also added back the coverage threshold and we're missing coverage somehow.

@kentcdodds
Copy link
Member

Ok, I think this should be good now 👍

@sheerun
Copy link
Contributor Author

sheerun commented Oct 12, 2018

yes, it'll be better to merge with 99% coverage and fix it later rather than rebase again soon

@kentcdodds
Copy link
Member

I think we're good. Time is funny. Pretty sure that this is ready to go. Let's just wait for CI.

@sheerun
Copy link
Contributor Author

sheerun commented Oct 12, 2018

I'll push fixed code coverage in a sec

@kentcdodds
Copy link
Member

We're all set :)

@kentcdodds kentcdodds merged commit 7be9a8b into testing-library:master Oct 12, 2018
@kentcdodds
Copy link
Member

🎉 This PR is included in version 3.11.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

kentcdodds pushed a commit that referenced this pull request Oct 12, 2018
This fixes code coverage of #125 by extracting helper functions.

There are still few istanbul ignores in code but they are not mine :)
@just-boris
Copy link

Hello, @sheerun. I see that you have added @sheerun/mutationobserver-shim dependency.

I would like to read the source code of this package, but it cannot be found on Github or elsewhere. The metadata in npm registry doesn't contain any link to source code.

Would you mind publishing source code somewhere to accept possible contributions?

@sheerun
Copy link
Contributor Author

sheerun commented Oct 17, 2018

Contributions should be submitted to the original repository but my fork is available at https://github.com/sheerun/MutationObserver.js

@sheerun
Copy link
Contributor Author

sheerun commented Oct 17, 2018

Ideally original repository would implement compatibility with commonjs so there's no need for a fork

@just-boris
Copy link

Do you have an open pull-request, that I can upvote?

@sheerun
Copy link
Contributor Author

sheerun commented Oct 17, 2018

no

@kentcdodds
Copy link
Member

Hey @sheerun, could you please open a pull request on the upstream repository? I think I'd like to move back to that original project rather than rely on a fork long-term.

@sheerun
Copy link
Contributor Author

sheerun commented Oct 23, 2018

sorry but preparing this PR will take considerable amount of time as tests are not working for me on original repository and switching main file to commonjs requires changes in compilation pipeline to preserve backward compatibility, simply I have no time for this. maybe someone can handle it

@kentcdodds
Copy link
Member

Understandable. @just-boris, would you be able to make such a PR?

@just-boris
Copy link

Yes, I was about to take care of it. I will post an update when it will actually happen.

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.

3 participants