Skip to content

Commit

Permalink
Merge pull request #19238 from emberjs/cleanup/remove-local-lookup
Browse files Browse the repository at this point in the history
[CLEANUP] Removes the `expandLocalLookup` API
  • Loading branch information
Chris Garrett authored Oct 29, 2020
2 parents 3727d0c + 473d15a commit 8f56dc8
Show file tree
Hide file tree
Showing 12 changed files with 20 additions and 740 deletions.
15 changes: 0 additions & 15 deletions FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,3 @@ for a detailed explanation.
* `ember-testing-resume-test`

Introduces the `resumeTest` testing helper to complement the `pauseTest` helper.

* `ember-module-unification`

Introduces support for Module Unification
([RFC](https://github.com/dgeb/rfcs/blob/module-unification/text/0000-module-unification.md))
to Ember. This includes:

- Passing the `source` of a `lookup`/`factoryFor` call as an argument to
`expandLocalLookup` on the resolver.
- Making `lookupComponentPair` friendly to local/private resolutions. The
new code ensures a local resolution is not paired with a global resolution.

This feature is paired with the
[`EMBER_RESOLVER_MODULE_UNIFICATION`](https://github.com/ember-cli/ember-resolver#ember_resolver_module_unification)
flag on the ember-resolver package.
24 changes: 3 additions & 21 deletions packages/@ember/-internals/container/lib/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,21 +214,14 @@ export default class Container {
@param {String} [options.source] The fullname of the request source (used for local lookup)
@return {any}
*/
factoryFor<T, C>(fullName: string, options: LookupOptions = {}): Factory<T, C> | undefined {
factoryFor<T, C>(fullName: string): Factory<T, C> | undefined {
if (this.isDestroyed) {
throw new Error(`Can not call \`.factoryFor\` after the owner has been destroyed`);
}
let normalizedName = this.registry.normalize(fullName);

assert('fullName must be a proper full name', this.registry.isValidFullName(normalizedName));

if (options.source || options.namespace) {
normalizedName = this.registry.expandLocalLookup(fullName, options);
if (!normalizedName) {
return;
}
}

return factoryFor<T, C>(this, normalizedName, fullName) as Factory<T, C> | undefined;
}
}
Expand Down Expand Up @@ -279,13 +272,6 @@ function isInstantiatable(container: Container, fullName: string) {
function lookup(container: Container, fullName: string, options: LookupOptions = {}) {
let normalizedName = fullName;

if (options.source || options.namespace) {
normalizedName = container.registry.expandLocalLookup(fullName, options);
if (!normalizedName) {
return;
}
}

if (options.singleton !== false) {
let cached = container.cache[normalizedName];
if (cached !== undefined) {
Expand Down Expand Up @@ -439,13 +425,9 @@ function processInjections(
let hash = result.injections;

for (let i = 0; i < injections.length; i++) {
let { property, specifier, source } = injections[i];
let { property, specifier } = injections[i];

if (source) {
hash[property] = lookup(container, specifier, { source });
} else {
hash[property] = lookup(container, specifier);
}
hash[property] = lookup(container, specifier);

if (!result.isDynamic) {
result.isDynamic = !isSingleton(container, specifier);
Expand Down
126 changes: 11 additions & 115 deletions packages/@ember/-internals/container/lib/registry.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,17 @@
import { Factory, LookupOptions } from '@ember/-internals/owner';
import { Factory } from '@ember/-internals/owner';
import { dictionary, intern } from '@ember/-internals/utils';
import { assert } from '@ember/debug';
import { assign } from '@ember/polyfills';
import { DEBUG } from '@glimmer/env';
import Container, { ContainerOptions, LazyInjection } from './container';

interface HasOptions {
source?: string;
namespace?: string;
}

export interface Injection {
namespace?: string;
property: string;
source?: string;
specifier: string;
}

export type ResolveOptions = {
specifier?: string;
source?: string | undefined;
namespace?: string | undefined;
};

export interface TypeOptions {
Expand All @@ -34,7 +25,6 @@ export interface KnownForTypeResult {

export interface IRegistry {
describe(fullName: string): string;
expandLocalLookup(fullName: string, options: LookupOptions): string;
getInjections(fullName: string): Injection[];
getOption<K extends keyof TypeOptions>(
fullName: string,
Expand All @@ -50,7 +40,6 @@ export interface IRegistry {
}

export type NotResolver = {
expandLocalLookup: never;
knownForType: never;
lookupDescription: never;
makeToString: never;
Expand All @@ -61,11 +50,6 @@ export type NotResolver = {
export type Resolve = <T, C>(name: string) => Factory<T, C> | undefined;

export interface Resolver {
expandLocalLookup?: (
normalizedName: string,
normalizedSource: string,
namespace?: string | undefined
) => string;
knownForType?: (type: string) => KnownForTypeResult;
lookupDescription?: (fullName: string) => string;
makeToString?: <T, C>(factory: Factory<T, C>, fullName: string) => string;
Expand Down Expand Up @@ -300,8 +284,8 @@ export default class Registry implements IRegistry {
@param {String} [options.source] the fullname of the request source (used for local lookups)
@return {Function} fullName's factory
*/
resolve<T, C>(fullName: string, options?: ResolveOptions): Factory<T, C> | undefined {
let factory = resolve<T, C>(this, this.normalize(fullName), options);
resolve<T, C>(fullName: string): Factory<T, C> | undefined {
let factory = resolve<T, C>(this, this.normalize(fullName));
if (factory === undefined && this.fallback !== null) {
factory = (this.fallback as any).resolve(...arguments);
}
Expand Down Expand Up @@ -393,15 +377,12 @@ export default class Registry implements IRegistry {
@param {String} [options.source] the fullname of the request source (used for local lookups)
@return {Boolean}
*/
has(fullName: string, options?: HasOptions) {
has(fullName: string) {
if (!this.isValidFullName(fullName)) {
return false;
}

let source = options && options.source && this.normalize(options.source);
let namespace = (options && options.namespace) || undefined;

return has(this, this.normalize(fullName), source, namespace);
return has(this, this.normalize(fullName));
}

/**
Expand Down Expand Up @@ -660,43 +641,6 @@ export default class Registry implements IRegistry {

return injections;
}

/**
Given a fullName and a source fullName returns the fully resolved
fullName. Used to allow for local lookup.
```javascript
let registry = new Registry();
// the twitter factory is added to the module system
registry.expandLocalLookup('component:post-title', { source: 'template:post' }) // => component:post/post-title
```
@private
@method expandLocalLookup
@param {String} fullName
@param {Object} [options]
@param {String} [options.source] the fullname of the request source (used for local lookups)
@return {String} fullName
*/
expandLocalLookup(fullName: string, options: LookupOptions) {
if (this.resolver !== null && this.resolver.expandLocalLookup) {
assert('fullName must be a proper full name', this.isValidFullName(fullName));
assert(
'options.source must be a proper full name',
!options.source || this.isValidFullName(options.source)
);

let normalizedFullName = this.normalize(fullName);
let normalizedSource = this.normalize(options.source!);

return expandLocalLookup(this, normalizedFullName, normalizedSource, options.namespace);
} else if (this.fallback !== null) {
return this.fallback.expandLocalLookup(fullName, options);
} else {
return null;
}
}
}

export declare class DebugRegistry extends Registry {
Expand All @@ -711,7 +655,7 @@ if (DEBUG) {

for (let key in hash) {
if (Object.prototype.hasOwnProperty.call(hash, key)) {
let { specifier, source, namespace } = hash[key];
let { specifier } = hash[key];
assert(
`Expected a proper full name, given '${specifier}'`,
this.isValidFullName(specifier)
Expand All @@ -720,8 +664,6 @@ if (DEBUG) {
injections.push({
property: key,
specifier,
source,
namespace,
});
}
}
Expand All @@ -735,56 +677,15 @@ if (DEBUG) {
}

for (let i = 0; i < injections.length; i++) {
let { specifier, source, namespace } = injections[i];
let { specifier } = injections[i];

assert(
`Attempting to inject an unknown injection: '${specifier}'`,
this.has(specifier, { source, namespace })
);
assert(`Attempting to inject an unknown injection: '${specifier}'`, this.has(specifier));
}
};
}

function expandLocalLookup(
registry: Registry,
normalizedName: string,
normalizedSource: string,
namespace: string | undefined
) {
let cache = registry._localLookupCache;
let normalizedNameCache = cache[normalizedName];

if (!normalizedNameCache) {
normalizedNameCache = cache[normalizedName] = Object.create(null);
}

let cacheKey = namespace || normalizedSource;

let cached = normalizedNameCache[cacheKey];

if (cached !== undefined) {
return cached;
}

let expanded = registry.resolver!.expandLocalLookup!(normalizedName, normalizedSource, namespace);

return (normalizedNameCache[cacheKey] = expanded);
}

function resolve<T, C>(
registry: Registry,
_normalizedName: string,
options?: ResolveOptions
): Factory<T, C> | undefined {
function resolve<T, C>(registry: Registry, _normalizedName: string): Factory<T, C> | undefined {
let normalizedName = _normalizedName;
// when `source` is provided expand normalizedName
// and source into the full normalizedName
if (options !== undefined && (options.source || options.namespace)) {
normalizedName = registry.expandLocalLookup(_normalizedName, options as LookupOptions);
if (!normalizedName) {
return;
}
}

let cached = registry._resolveCache[normalizedName];
if (cached !== undefined) {
Expand Down Expand Up @@ -813,13 +714,8 @@ function resolve<T, C>(
return resolved;
}

function has(
registry: Registry,
fullName: string,
source: string | undefined,
namespace: string | undefined
) {
return registry.resolve(fullName, { source, namespace }) !== undefined;
function has(registry: Registry, fullName: string) {
return registry.resolve(fullName) !== undefined;
}

const privateNames: { [key: string]: string } = dictionary(null);
Expand Down
21 changes: 0 additions & 21 deletions packages/@ember/-internals/container/tests/container_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -513,27 +513,6 @@ moduleFor(
Service.create(result);
}

['@test lookup passes options through to expandlocallookup'](assert) {
let registry = new Registry();
let container = registry.container();
let PostController = factory();

registry.register('controller:post', PostController);
registry.expandLocalLookup = (fullName, options) => {
assert.ok(true, 'expandLocalLookup was called');
assert.equal(fullName, 'foo:bar');
assert.deepEqual(options, { source: 'baz:qux' });

return 'controller:post';
};

let PostControllerLookupResult = container.lookup('foo:bar', {
source: 'baz:qux',
});

assert.ok(PostControllerLookupResult instanceof PostController);
}

['@test #factoryFor class is registered class'](assert) {
let registry = new Registry();
let container = registry.container();
Expand Down
Loading

0 comments on commit 8f56dc8

Please sign in to comment.