|
| 1 | +- Start Date: 2015-04-09 |
| 2 | +- RFC PR: https://github.com/emberjs/rfcs/pull/46 |
| 3 | +- Ember Issue: https://github.com/emberjs/ember.js/pull/11440 |
| 4 | + |
| 5 | +# Summary |
| 6 | + |
| 7 | +Fully encapsulate and privatize the `Container` and `Registry` classes by |
| 8 | +exposing a select subset of public methods on `Application` and |
| 9 | +`ApplicationInstance`. |
| 10 | + |
| 11 | +# Motivation |
| 12 | + |
| 13 | +The `Container` and `Registry` classes currently lead a confusing life of |
| 14 | +semi-private exclusion within Ember applications. They are undocumented |
| 15 | +publicly but not fully private either, as knowledge of their particulars is |
| 16 | +required for developing both initializers and unit tests. This situation has |
| 17 | +become untenable as the new `Registry` class has been extracted from |
| 18 | +`Container`, and the complexity of their usage has grown across |
| 19 | +`Application` and `ApplicationInstance` classes. |
| 20 | + |
| 21 | +We can bring sanity to this situation by continuing the work started at the |
| 22 | +`Application` level to expose methods such as `register` and `inject` from the |
| 23 | +internally maintained `Registry`. |
| 24 | + |
| 25 | +Furthermore, once `Container` and `Registry` are fully private, their |
| 26 | +architecture and documentation can be cleaned up. For instance, a |
| 27 | +`Container` can freely reference its associated `Registry` as `registry` |
| 28 | +rather than `_registry`, as it can be assumed that only framework developers |
| 29 | +will reference this property. |
| 30 | + |
| 31 | +# Detailed design |
| 32 | + |
| 33 | +`Application` will expose the following methods from its internally maintained |
| 34 | +registry: |
| 35 | + |
| 36 | +* `register` |
| 37 | +* `inject` |
| 38 | +* `registerOptions` - mapped to `Registry#options` |
| 39 | +* `registerOptionsForType` - mapped to `Registry#optionsForType` |
| 40 | + |
| 41 | +`ApplicationInstance` will also expose the the same methods. However, these |
| 42 | +methods will be exposed from its own internally maintained registry, which |
| 43 | +has the associated `Application`'s registry configured as a "fall back". No |
| 44 | +direct path will be provided from the `ApplicationInstance` to the |
| 45 | +`Application`'s registry. |
| 46 | + |
| 47 | +`ApplicationInstance` will also expose the following methods from its |
| 48 | +internally maintained container: |
| 49 | + |
| 50 | +* `lookup` |
| 51 | +* `lookupFactory` |
| 52 | + |
| 53 | +`ApplicationInstance` will cease exposing `container`, `registry`, and |
| 54 | +`applicationRegistry` publicly. |
| 55 | + |
| 56 | +`Application` initializers will receive a single argument to `initialize`: |
| 57 | +`application`. |
| 58 | + |
| 59 | +Likewise, `ApplicationInstance` initializers will receive a single argument |
| 60 | +to `initialize`: `applicationInstance`. |
| 61 | + |
| 62 | +`Container` and `Registry` will be made fully private and documented as |
| 63 | +such. Each `Container` will freely reference its associated `Registry` as |
| 64 | +`registry` rather than `_registry`. |
| 65 | + |
| 66 | +[ember-test-helpers](https://github.com/switchfly/ember-test-helpers) |
| 67 | +will provide an `isolatedApplicationInstance` method instead of an |
| 68 | +`isolatedContainer` for unit testing. A mechanism will be developed to specify |
| 69 | +which initializers should be engaged in the initialization of this instance. |
| 70 | +In this way, we can avoid duplication of registration logic, as is currently |
| 71 | +done in a most un-DRY manner in the [isolatedContainer](https://github.com/switchfly/ember-test-helpers/blob/master/lib/ember-test-helpers/isolated-container.js#L56-L79). |
| 72 | + |
| 73 | +# Drawbacks |
| 74 | + |
| 75 | +This refactor will require maintaining backwards compatibility and |
| 76 | +deprecation warnings until Ember 2.0. This will temporarily increase |
| 77 | +internal code complexity and file sizes. |
| 78 | + |
| 79 | +# Alternatives |
| 80 | + |
| 81 | +The obvious alternative is to make `Container` and `Registry` fully public |
| 82 | +and documented. An application's registry would be available as a `registry` |
| 83 | +property. An application instance's container would remain available as |
| 84 | +`container`. |
| 85 | + |
| 86 | +We could still pass an `Application` into application initializers |
| 87 | +and an `ApplicationInstance` into application instance initializers. |
| 88 | + |
| 89 | +If this alternative is taken, I would suggest that `Application` should |
| 90 | +deprecate `register` and `inject` in favor of calling the equivalents on its |
| 91 | +public `registry`. |
| 92 | + |
| 93 | +Regardless of which alternative is chosen, we should ensure that the public |
| 94 | +aspects of container and registry usage are well documented. |
| 95 | + |
| 96 | +# Unresolved questions |
| 97 | + |
| 98 | +* Are the public methods listed above sufficient or should any others be |
| 99 | +exposed? |
| 100 | + |
| 101 | +* What mechanism should be used to engage initializers in unit and |
| 102 | +integration tests? Should test modules simply have an `initializers` array, |
| 103 | +similar to the current `needs` array? |
| 104 | + |
| 105 | +* Given the semi-private nature of containers and registries, we may not need |
| 106 | +to worry about semver for deprecations. However, we should be good citizens |
| 107 | +and properly deprecate as much as possible. Some real world use cases in |
| 108 | +initializers will no doubt be a surprise, so we need to tread carefully. |
0 commit comments