Skip to content
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

defaultType support, mv partials code to resolver #192

Merged
merged 1 commit into from
Jun 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions mu-trees/addon/ember-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export default function generateConfig(name) {
}
},
transform: { definitiveCollection: 'transforms' },
util: { definitiveCollection: 'utils' },
view: { definitiveCollection: 'views' },
'-view-registry': { definitiveCollection: 'main' },
'-bucket-cache': { definitiveCollection: 'main' }
Expand All @@ -47,40 +46,55 @@ export default function generateConfig(name) {
},
components: {
group: 'ui',
privateCollections: ['utils'],
types: ['component', 'helper', 'template']
},
initializers: {
group: 'init',
defaultType: 'initializer',
privateCollections: ['utils'],
types: ['initializer']
},
'instance-initializers': {
group: 'init',
defaultType: 'instance-initializer',
privateCollections: ['utils'],
types: ['instance-initializers']
},
models: {
group: 'data',
defaultType: 'model',
privateCollections: ['utils'],
types: ['model', 'adapter', 'serializer']
},
partials: {
group: 'ui',
defaultType: 'partial',
privateCollections: ['utils'],
types: ['partial']
},
routes: {
group: 'ui',
privateCollections: ['components'],
privateCollections: ['components', 'utils'],
types: ['route', 'controller', 'template']
},
services: {
defaultType: 'service',
privateCollections: ['utils'],
types: ['service']
},
utils: {
unresolvable: true
},
views: {
defaultType: 'view',
privateCollections: ['utils'],
types: ['view']
},
transforms: {
group: 'data',
defaultType: 'transform',
privateCollections: ['utils'],
types: ['transform']
}
}
Expand Down
71 changes: 44 additions & 27 deletions mu-trees/addon/module-registries/requirejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,7 @@ export default class RequireJSRegistry {
this._require = require;
}

_normalize(specifier) {
let s = deserializeSpecifier(specifier);

// This is hacky solution to get around the fact that Ember
// doesn't know it is requesting a partial. It requests something like
// 'template:/my-app/routes/-author'
// Would be better to request 'template:my-app/partials/author'
let isPartial = s.type === 'template' && s.name[0] === '-';
if (isPartial) {
s.name = s.name.slice(1);
s.collection = 'partials';
}

_baseSegments(s) {
let collectionDefinition = this._config.collections[s.collection];
let group = collectionDefinition && collectionDefinition.group;
let segments = [ s.rootName, this._modulePrefix ];
Expand Down Expand Up @@ -52,25 +40,54 @@ export default class RequireJSRegistry {
segments.push(s.name);
}

if (!isPartial) {
segments.push(s.type);
}
return segments;
}

_detectModule(specifierString, lookupMethod) {
let specifier = deserializeSpecifier(specifierString);

let segments = this._baseSegments(specifier);
let basePath = `${segments.join('/')}`;
let typedPath = `${basePath}/${specifier.type}`;

let path = segments.join('/');
let lookupResult = lookupMethod(typedPath);

return path;
if (
!lookupResult &&
this._config.collections[specifier.collection].defaultType === specifier.type
) {
lookupResult = lookupMethod(basePath);
}

return lookupResult;
}

has(specifier) {
let path = this._normalize(specifier);
// Worth noting this does not confirm there is a default export,
// as would be expected with this simple implementation of the module
// registry.
return path in this._require.entries;
has(specifierString) {
return this._detectModule(specifierString, path => {
/*
* Worth noting this does not confirm there is a default export,
* as would be expected with this simple implementation of the module
* registry.
*
* To preserve sanity, the `get` method throws when a `default`
* export is not found.
*/
return path in this._require.entries;
});
}

get(specifier) {
let path = this._normalize(specifier);
return this._require(path).default;
get(specifierString) {
let module = this._detectModule(specifierString, path => {
return (path in this._require.entries) && this._require(path);
});

if (!module) {
return module;
}

if (!module.default) {
throw new Error('RequireJSRegistry expects all resolved modules to have a default export.');
}
return module.default;
}
}
27 changes: 27 additions & 0 deletions mu-trees/addon/resolvers/glimmer-wrapper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import RequireJSRegistry from '../../module-registries/requirejs';

const { DefaultResolver } = Ember;

const TEMPLATE_TO_PARTIAL = /^template:(.*\/)?_([\w-]+)/;

/*
* Wrap the @glimmer/resolver in Ember's resolver API. Although
* this code extends from the DefaultResolver, it should never
Expand All @@ -23,11 +25,36 @@ const Resolver = DefaultResolver.extend({
normalize: null,

resolve(lookupString) {
/*
* Ember paritals are looked up as templates. Here we replace the template
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

* resolution with a partial resolute when appropriate. Try to keep this
* code as "pay-go" as possible.
*/

if (lookupString.indexOf('template:') === 0) {
lookupString = this._templateToPartial(lookupString);
}
return this._resolve(lookupString);
},

_resolve(lookupString) {
return this._glimmerResolver.resolve(lookupString);
},

/*
* templates may actually be partial lookups, so consider them as possibly
* such and return the correct lookupString.
*/
_templateToPartial(lookupString) {
let match = TEMPLATE_TO_PARTIAL.exec(lookupString);
if (!match) {
return lookupString;
}

let namespace = match[1] || '';
let name = match[2];

return `partial:${namespace}${name}`;
}
});

Expand Down
55 changes: 49 additions & 6 deletions mu-trees/tests/unit/module-registries/requirejs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ export let config = {
types: [ 'template' ]
},
routes: {
defaultType: 'route',
group: 'ui',
privateCollections: ['components'],
types: ['route', 'controller', 'template']
},
services: {
defaultType: 'service',
types: ['service']
}
}
Expand All @@ -51,32 +53,73 @@ function buildMockRequire() {
module('RequireJS Registry', {
beforeEach() {
this.mockRequire = buildMockRequire();
this.mockRequire.entries = {};
this.config = config;
this.registry = new RequireJSRegistry(this.config, 'src', this.mockRequire);
},

addModule(name, module) {
this.mockRequire.entries[name] = module;
}

});

test('Normalize', function(assert) {
assert.expect(10);
test('basic get', function(assert) {
assert.expect(8);

[
/*
* Over time lets move these general cases into specific tests that
* describe their aim.
*/
[ 'router:/my-app/main/main', 'my-app/src/router' ],
[ 'route:/my-app/routes/application', 'my-app/src/ui/routes/application/route' ],
[ 'template:/my-app/routes/application', 'my-app/src/ui/routes/application/template' ],
[ 'component:/my-app/components/my-input', 'my-app/src/ui/components/my-input/component' ],
[ 'template:/my-app/routes/components/my-input', 'my-app/src/ui/components/my-input/template' ],
[ 'template:/my-app/components/my-input', 'my-app/src/ui/components/my-input/template' ],
[ 'component:/my-app/components/my-input/my-button', 'my-app/src/ui/components/my-input/my-button/component' ],
[ 'template:/my-app/components/my-input/my-button', 'my-app/src/ui/components/my-input/my-button/template' ],
[ 'template:/my-app/routes/-author', 'my-app/src/ui/partials/author' ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the other changes in this PR make partials still work, why remove the test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment about this in the source- I'd like to see this bulk test be simmered down into smaller focused tests with better descriptions of the intent. @iezer and I took the first steps toward some utilities to help keep those test terse.

[ 'service:/my-app/services/auth', 'my-app/src/services/auth/service' ]
[ 'template:/my-app/components/my-input/my-button', 'my-app/src/ui/components/my-input/my-button/template' ]
]
.forEach(([ lookupString, expected ]) => {
let expectedModule = {};
this.mockRequire.entries = {
[expected]: {default: expectedModule}
};
let actualModule = this.registry.get(lookupString);
assert.equal(actualModule, expectedModule, `normalize ${lookupString} -> ${expected}`);
assert.equal(actualModule, expectedModule, `get ${lookupString} -> ${expected}`);
});
});

test('typed module name with default export', function(assert) {
let expectedModule = {};
this.addModule(`my-app/src/ui/routes/index/route`, {default: expectedModule});

let actualModule = this.registry.get(`route:/my-app/routes/index`);
assert.equal(
actualModule, expectedModule,
`resolved the module`
);
});

test('un-typed module name with default export when resolved type is the defaultType', function(assert) {
let expectedModule = {};
this.addModule(`my-app/src/ui/routes/index`, {default: expectedModule});

let actualModule = this.registry.get(`route:/my-app/routes/index`);
assert.equal(
actualModule, expectedModule,
`resolved the module`
);
});

test('un-typed module name with default export when resolved type is not the defaultType', function(assert) {
let expectedModule = {};
this.addModule(`my-app/src/ui/routes/index`, {default: expectedModule});

let actualModule = this.registry.get(`template:/my-app/routes/index`);
assert.notOk(
actualModule,
`did not resolve the module`
);
});
27 changes: 27 additions & 0 deletions mu-trees/tests/unit/resolvers/glimmer-wrapper/basic-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,30 @@ test('Can resolve a top level template of a definitive type', function(assert) {
'relative module specifier with source resolved'
);
});

test('Can resolve a partial', function(assert) {
let template = {};
let resolver = this.resolverForEntries({
app: {
name: 'example-app'
},
types: {
partial: { definitiveCollection: 'partials' }
},
collections: {
partials: {
group: 'ui',
defaultType: 'partial',
types: ['partial']
}
}
}, {
'partial:/app/partials/author': template
});

assert.equal(
resolver.resolve('template:_author', ''),
template,
'partial resolved'
);
});