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

Check for owner before calling isGlimmerTwo #846

Closed
wants to merge 2 commits into from

Conversation

RobbieTheWagner
Copy link
Member

Resolves #816

This stops the errors from happening, when there is no owner, because now we are checking if we have an owner before trying to use the owner.

I am not sure if this is the correct solution or if there is a way to actually get the real owner again here, rather than just doing nothing if it does not exist.

@teddyzeenny do we want to support running the inspector on localhost:4200/tests anyway?

@Bestra
Copy link
Contributor

Bestra commented Aug 3, 2018

@rwwagner90 I'd say what we've got in this PR is at least an improvement in the interim even we want to settle on different semantics longterm. I'd assumed that we always had an owner before this, so making us aware of the possible null (thanks for the code comments!) is a good thing. I vote :shipit: for now.

@RobbieTheWagner
Copy link
Member Author

It seems we used to have an owner during tests, but maybe not anymore? Perhaps something changed in Ember 3.1+?

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

2 participants