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

Make DOM utilities work in nested browsing contexts #222

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

acusti
Copy link
Contributor

@acusti acusti commented Mar 16, 2017

This is a PR of #188 (which was merged into master in October) into the fbjs-0.8.x branch. I’ve been trying to freshen up http://github.com/facebook/react/pull/7936 (wound up having to just create a new branch from master with http://github.com/facebook/react/pull/9184) and make sure it’s ready to go and was noticing that the tests I’d added were hanging, then I realized it’s because the changes in fbjs that the React PR depends on (#188) haven’t been released to the fbjs 0.8.x line.

I also see from #217 that no React 15.x release will upgrade to fbjs 0.9.x because it will entail a breaking change and therefore a new major release. So, can we just merge the updated functionality into fbjs-0.8.x and release a new version there?

Let me know any concerns. The API changes in this PR are 100% compatible with previous versions and the functionality changes were designed to not affect existing code in any way.

* [fbjs] Use relative document and window (facebook#156)

* [fbjs] Support nested browsing contexts (facebook#156)

* [fbjs] Test getActiveElement api change (facebook#156)
@zpao
Copy link
Member

zpao commented Mar 16, 2017

Thanks @acusti!

cc @spicyj @gaearon

I don't think there should be any issue shipping this out in 0.8 but that will mean that React 15.* will get this automatically (which means further small differences between the npm version and the dist version).

Let me know if you have any concerns, otherwise I'll ship this soon.

@sophiebits
Copy link
Contributor

ack 👍

@zpao zpao merged commit c8c794d into facebook:fbjs-0.8.x Mar 21, 2017
@zpao
Copy link
Member

zpao commented Mar 21, 2017

Just published v0.8.10. Thanks again!

@ramesius
Copy link

ramesius commented Mar 21, 2017

@zpao Hey, we are seeing failures in our tests where we shallow render our components. Looks like it stems from this change, the removal of the following guard on document: if (typeof document === 'undefined'), is causing ReferenceError: document is not defined. Whatever is calling getActiveElement() is not passing in doc, and since it is running in node there is no global document.

@danez
Copy link

danez commented Mar 21, 2017

I have the same here in our tests and serverside rendering. @zpao

@acusti
Copy link
Contributor Author

acusti commented Mar 21, 2017

Yikes. The test / server-side rendering failures should be resolved by #223. @ramesius thanks for the prompt note and thorough explanation!

@ramesius
Copy link

@acusti Thanks for the quick turn around on this one.

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

Successfully merging this pull request may close these issues.

6 participants