-
Notifications
You must be signed in to change notification settings - Fork 292
fix loading race condition #2689
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
Conversation
ef5c844 to
a9dec4b
Compare
mansona
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed in the office hours: I'm not comfortable making a change like this without first doing the work to create multiple test apps that are separate from the ember-inspector code itself
a9dec4b to
4db85de
Compare
|
@mansona added a failing test |
9df396f to
74ef05c
Compare
|
I agree that we need separate tests (happy to pair on this). But i think we should not wait for it to fix current issues. |
NullVoxPopuli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me -- makes sense -- all the adapter imports are now non-eager
f3ff154 to
ebb0432
Compare
ebb0432 to
d81cf02
Compare
NullVoxPopuli
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for slimming down this PR to just the entrypoints changes!
Description
loadEmberDebugInWebpage needs to wrap the whole loading part. as it was before last release.
this ensures that ember is available before trying to access anything from it.
fixes #2688
Screenshots