[BUGFIX beta] Check that handler exists before triggering event

[trentmwillis]

This is necessary for lazily-loaded routes where the handler is not
available synchronously. This includes events like loading and
queryParamsDidChange which trigger synchronously after starting a
transition, in those cases we should by-pass handlers that are not
yet loaded.

cc/ @rwjblue @nathanhammond @dgeb

Note: I feel like I should add a test for this, but I feel like it makes more
sense to just test this in the ember-engines repo, since this code path is
impossible to hit normally.

[dgeb]

LGTM. I think I'm fine merging without an additional test.

[dgeb]

@trentmwillis could you please prefix the commit message with [BUGFIX beta] ?

[nathanhammond]

The other approach we considered was a promise chain to guarantee that the handler was already loaded, but that was breaking everything in Ember and doesn't actually change the behavior here.

[trentmwillis]

@dgeb updated. I'm always unsure of what to prefix my commits with.

[krisselden]

@dgeb I have doubts about this not being tested.

[dgeb]

@krisselden That's fair. Let's hold off on merging until we can figure out the best way to test this.

[trentmwillis]

Added two unit tests, let me know if you think more is needed. Opted for unit tests since the triggerEvent function is just an implementation of a hook from Router.js.

[rwjblue]

These tests seem good for the changes being made here.

I would like to make sure that we get at least one higher level acceptance style test that emulates what we are actually doing in ember-engines WRT to getHandler returning a promise. That doesn't need to block this PR per-se, but it seems fairly important to help avoid accidental regressions here...

[trentmwillis]

@rwjblue I agree. Unfortunately, having getHandler return a Promise doesn't work without also modifying the default query params behavior, which is something I don't think we want to upstream yet since we haven't quite tested it out in the real world.

The only reason this PR is happening right now is because it seemed to make more sense than trying to monkeypatch it into the addon.

[rwjblue]

@trentmwillis - Would you mind making a followup issue here to add an acceptance test for getHandler returning a promise?

[trentmwillis]

Opened an issue to follow up.