Simplify engine

[scalvert]

@rwjblue this change supersedes #200 and is dependent on emberjs/ember-test-helpers#177

[chadhietala]

This is going to fail until the test-helpers land.

[rwjblue]

I'm generally not in favor of this approach. I do not believe we need to add more per-addon customizations to ember-test-helpers. If the resolver is setup properly, it will automatically find our custom link-to and link-to-external components.

[rwjblue]

This is unused

[scalvert]

I'm using it on line 10. An old commit wasn't using it, but it's used now!

[trentmwillis]

If the resolver is setup properly, it will automatically find our custom link-to and link-to-external components.

This is true, but is this really the best approach? We don't use the resolver to find the other constructs which are explicitly registered (at least to my knowledge). That said, I'm not particularly fond of adding addon-specific code into the test helpers either.

Maybe ember-test-helpers could expose a hook to allow setting up registrations easily?

[rwjblue]

This is true, but is this really the best approach? We don't use the resolver to find the other constructs which are explicitly registered (at least to my knowledge).

The other constructs are provided by Ember itself, which is not in the addon ecosystem so it has no other choice (also legacy/global land).

Maybe ember-test-helpers could expose a hook to allow setting up registrations easily?

Perhaps, but we would have the same issue we have right now (that @scalvert and I have been poking at today separately from this PR) which is that we would need a way to kick off our custom code to register with ember-test-helpers. We could ask that users do this manually in projects (i.e. in tests/test-helper.js or something), but I think we can do better...

---

Assuming that the interest here is in solving #124, the easiest solution is to not need a custom registry. That is what my point was above, if we setup the modules in the right place the standard resolver system would "just work".

[trentmwillis]

All fair points. So would using a resolver, mean bringing back ember-engines/resolver? That could definitely work and coupled with the recent custom resolver support in ember-test-helpers, should be easily workable for standalone engines and apps with several in-repo engines (such as ours).

Edit: nevermind, I think I just figured out what you meant by "setup the modules in the right place". So have modules like app/components/link-to and app/components/link-to-external?

[rwjblue]

So have modules like app/components/link-to and app/components/link-to-external?

Exactly! I did initial work for this in https://github.com/dgeb/ember-engines/compare/simplify-link-to (specifically add141a), which was based on top of #200 but I wanted to land #200 first before continuing.

[scalvert]

So that makes sense to me. The reason @chadhietala and I opted to move to this approach was, after speaking with @stefanpenner, we thought the correct direction was for host apps to explicitly import this behavior when adding engines. Using the import with side-effects in app.js inside the host app would allow for us to remove the guesswork as to the call order. This way, we can guarantee when this code is run, rather than hoping it would be run at the correct time.

Does this make sense?

[stefanpenner]

@rwjblue the thought was, we can dress it up nicer later, but playing a preprend/postpend of files arms race should be avoided.

[scalvert]

So which direction do we want to take here? It seems we have a few approaches:

1. The original solution @rwjblue and I discussed, which involved adding the *-ext files to vendor (issues around lack of order of those files' positions within vendor were identified)
2. The approach @chadhietala and I suggested in this PR, which exports the component registration portion in a function that's exposed for the host application to use. This allows for the host application to invoke the registration from within app.js

@rwjblue I realize you've made further changes in https://github.com/dgeb/ember-engines/compare/simplify-link-to. I'll defer to you guys as the subject matter experts.

One quick question for @rwjblue and @trentmwillis: https://github.com/dgeb/ember-engines/blob/master/addon/-private/engine-ext.js#L12 seems to imply that we only want to register these components if we're not in the scope of an Application, and the proposal to export these components in app seem to imply that we'll hoist these components to the app namespace. Are these components intended to be scoped within engines only?

[trentmwillis]

I could be wrong, but I think we're conflating several issues.

• [WIP] Simplify engine exports. #200, overall, was just about cleaning up how everything worked and not relying on private APIs as heavily.
• The vendor ordering was to address the issue I raised about the reopens not occurring early enough, which prevents [WIP] Simplify engine exports. #200 from landing.
• The registration function seems to address the link-to-external in integration tests issue.

It seems like the primary issue that @scalvert is concerned with is the integration testing issue, in which case I think @rwjblue's proposed solution makes sense (though with some caveats).

The vendor ordering seems to address the reopening issue, though I concede it is fairly brittle with regards to ordering. It seems to me like we can figure that out separately from the link-to-external issue to avoid muddying the waters and to keep the PRs smaller.

[scalvert]

Agree the issues have been entangled.

It seems like the primary issue that @scalvert is concerned with is the integration testing issue, in which case I think @rwjblue's proposed solution makes sense (though with some caveats).

Which proposed solution were you referring to? The solution where we output the initialization of the engine extension files into vendor directly via broccoli? Or, exposing the components by exporting from app?

[scalvert]

Closing so we can decouple the fixes.