Skip to content

Commit 7ffb39c

Browse files
NullVoxPopuliclaude
andcommitted
Address review: inflection, colocation, drop unused helpers
Review feedback on the strict resolver boiled down to three things: proper pluralization with customization, nested-colocation component lookup, and dead code in strict-resolver/string.js. Pluralization: #plural now handles -s/-ss/-sh/-ch/-x/-z suffixes, consonant + y, and the common irregulars (child/children, person/people, man/men, woman/women, mouse/mice, tooth/teeth, foot/feet). The plurals constructor option still overrides anything, so a user can opt their "childs" type back in. Nested colocation: added #nestedColocationLookup so component:my-widget resolves to ./components/my-widget/index.{js,ts,gjs,gts} — the common pattern for a class paired with a template in a folder. Direct module matches still take precedence. Dead code: classify and underscore weren't used by the resolver or anything else in ember-source, so they're gone (along with their test files). decamelize is collapsed inside dasherize's implementation. The setStrings/getStrings/getString runtime string table was also unused; removed. dasherize loses its yuidoc @method tag since the same name is already documented via @ember/-internals/string. Tests: cover -s/-es suffix rules, consonant-y, irregular plurals, custom plural overriding an irregular, and the three colocation cases (component/helper/modifier, plus direct-wins-over-colocation). tests/docs/expected.js drops the entries whose only source was the trimmed strict-resolver/string.js. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a89a953 commit 7ffb39c

7 files changed

Lines changed: 139 additions & 248 deletions

File tree

packages/@ember/engine/lib/strict-resolver.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,18 @@ export class StrictResolver implements Resolver {
3030
}
3131

3232
#plural(s: string) {
33-
return this.#plurals.get(s) ?? s + 's';
33+
return this.#plurals.get(s) ?? pluralize(s);
3434
}
3535

3636
resolve(fullName: string): Factory<object> | object | undefined {
3737
let [type, name] = fullName.split(':') as [string, string];
3838
name = this.#normalizeName(type, name);
39-
for (let strategy of [this.#resolveSelf, this.#mainLookup, this.#defaultLookup]) {
39+
for (let strategy of [
40+
this.#resolveSelf,
41+
this.#mainLookup,
42+
this.#defaultLookup,
43+
this.#nestedColocationLookup,
44+
]) {
4045
let result = strategy.call(this, type, name);
4146
if (result) {
4247
return this.#extractDefaultExport(result.hit);
@@ -101,6 +106,50 @@ export class StrictResolver implements Resolver {
101106
}
102107
return undefined;
103108
}
109+
110+
// Supports the nested colocation pattern where `component:my-widget`
111+
// resolves to `./components/my-widget/index.{js,ts,gjs,gts}`. The index
112+
// file is typically the component class, and it's commonly paired with a
113+
// sibling `template.hbs` inside the same folder.
114+
#nestedColocationLookup(type: string, name: string): Result {
115+
let dir = this.#plural(type);
116+
let target = `${dir}/${name}/index`;
117+
let module = this.#modules.get(target);
118+
if (module) {
119+
return { hit: module };
120+
}
121+
return undefined;
122+
}
123+
}
124+
125+
// Handle the common irregular English plurals plus the standard -s / -es
126+
// suffix rules. Users can override any type via the `plurals` constructor
127+
// option (including overriding these defaults).
128+
const IRREGULAR_PLURALS: Record<string, string> = Object.freeze({
129+
child: 'children',
130+
man: 'men',
131+
woman: 'women',
132+
person: 'people',
133+
mouse: 'mice',
134+
tooth: 'teeth',
135+
foot: 'feet',
136+
});
137+
138+
const NEEDS_ES_SUFFIX = /(s|ss|sh|ch|x|z)$/;
139+
const ENDS_IN_CONSONANT_Y = /([^aeiou])y$/;
140+
141+
function pluralize(singular: string): string {
142+
let irregular = IRREGULAR_PLURALS[singular];
143+
if (irregular) {
144+
return irregular;
145+
}
146+
if (ENDS_IN_CONSONANT_Y.test(singular)) {
147+
return singular.replace(ENDS_IN_CONSONANT_Y, '$1ies');
148+
}
149+
if (NEEDS_ES_SUFFIX.test(singular)) {
150+
return singular + 'es';
151+
}
152+
return singular + 's';
104153
}
105154

106155
const fileExtension = /\.\w{1,4}$/;
Lines changed: 5 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,127 +1,16 @@
1-
/* eslint-disable no-useless-escape */
21
import Cache from './cache';
3-
let STRINGS = {};
4-
export function setStrings(strings) {
5-
STRINGS = strings;
6-
}
7-
export function getStrings() {
8-
return STRINGS;
9-
}
10-
export function getString(name) {
11-
return STRINGS[name];
12-
}
2+
133
const STRING_DASHERIZE_REGEXP = /[ _]/g;
14-
const STRING_DASHERIZE_CACHE = new Cache(1000, (key) =>
15-
decamelize(key).replace(STRING_DASHERIZE_REGEXP, '-')
16-
);
17-
const STRING_CLASSIFY_REGEXP_1 = /^(\-|_)+(.)?/;
18-
const STRING_CLASSIFY_REGEXP_2 = /(.)(\-|\_|\.|\s)+(.)?/g;
19-
const STRING_CLASSIFY_REGEXP_3 = /(^|\/|\.)([a-z])/g;
20-
const classifyReplace1 = (_match, _separator, chr) => (chr ? `_${chr.toUpperCase()}` : '');
21-
const classifyReplace2 = (_match, initialChar, _separator, chr) =>
22-
initialChar + (chr ? chr.toUpperCase() : '');
23-
const CLASSIFY_CACHE = new Cache(1000, (str) => {
24-
let parts = str.split('/');
25-
for (let i = 0; i < parts.length; i++) {
26-
parts[i] = parts[i]
27-
.replace(STRING_CLASSIFY_REGEXP_1, classifyReplace1)
28-
.replace(STRING_CLASSIFY_REGEXP_2, classifyReplace2);
29-
}
30-
return parts.join('/').replace(STRING_CLASSIFY_REGEXP_3, (match) => match.toUpperCase());
31-
});
32-
const STRING_UNDERSCORE_REGEXP_1 = /([a-z\d])([A-Z]+)/g;
33-
const STRING_UNDERSCORE_REGEXP_2 = /\-|\s+/g;
34-
const UNDERSCORE_CACHE = new Cache(1000, (str) =>
35-
str
36-
.replace(STRING_UNDERSCORE_REGEXP_1, '$1_$2')
37-
.replace(STRING_UNDERSCORE_REGEXP_2, '_')
38-
.toLowerCase()
39-
);
404
const STRING_DECAMELIZE_REGEXP = /([a-z\d])([A-Z])/g;
5+
416
const DECAMELIZE_CACHE = new Cache(1000, (str) =>
427
str.replace(STRING_DECAMELIZE_REGEXP, '$1_$2').toLowerCase()
438
);
44-
/**
45-
Converts a camelized string into all lower case separated by underscores.
46-
47-
```javascript
48-
import { decamelize } from '@ember/string';
49-
50-
decamelize('innerHTML'); // 'inner_html'
51-
decamelize('action_name'); // 'action_name'
52-
decamelize('css-class-name'); // 'css-class-name'
53-
decamelize('my favorite items'); // 'my favorite items'
54-
```
55-
56-
@method decamelize
57-
@param {String} str The string to decamelize.
58-
@return {String} the decamelized string.
59-
@private
60-
*/
61-
export function decamelize(str) {
62-
return DECAMELIZE_CACHE.get(str);
63-
}
64-
/**
65-
Replaces underscores, spaces, or camelCase with dashes.
669

67-
```javascript
68-
import { dasherize } from '@ember/string';
69-
70-
dasherize('innerHTML'); // 'inner-html'
71-
dasherize('action_name'); // 'action-name'
72-
dasherize('css-class-name'); // 'css-class-name'
73-
dasherize('my favorite items'); // 'my-favorite-items'
74-
dasherize('privateDocs/ownerInvoice'; // 'private-docs/owner-invoice'
75-
```
10+
const STRING_DASHERIZE_CACHE = new Cache(1000, (key) =>
11+
DECAMELIZE_CACHE.get(key).replace(STRING_DASHERIZE_REGEXP, '-')
12+
);
7613

77-
@method dasherize
78-
@param {String} str The string to dasherize.
79-
@return {String} the dasherized string.
80-
@public
81-
*/
8214
export function dasherize(str) {
8315
return STRING_DASHERIZE_CACHE.get(str);
8416
}
85-
/**
86-
Returns the UpperCamelCase form of a string.
87-
88-
```javascript
89-
import { classify } from '@ember/string';
90-
91-
classify('innerHTML'); // 'InnerHTML'
92-
classify('action_name'); // 'ActionName'
93-
classify('css-class-name'); // 'CssClassName'
94-
classify('my favorite items'); // 'MyFavoriteItems'
95-
classify('private-docs/owner-invoice'); // 'PrivateDocs/OwnerInvoice'
96-
```
97-
98-
@method classify
99-
@param {String} str the string to classify
100-
@return {String} the classified string
101-
@public
102-
*/
103-
export function classify(str) {
104-
return CLASSIFY_CACHE.get(str);
105-
}
106-
/**
107-
More general than decamelize. Returns the lower\_case\_and\_underscored
108-
form of a string.
109-
110-
```javascript
111-
import { underscore } from '@ember/string';
112-
113-
underscore('innerHTML'); // 'inner_html'
114-
underscore('action_name'); // 'action_name'
115-
underscore('css-class-name'); // 'css_class_name'
116-
underscore('my favorite items'); // 'my_favorite_items'
117-
underscore('privateDocs/ownerInvoice'); // 'private_docs/owner_invoice'
118-
```
119-
120-
@method underscore
121-
@param {String} str The string to underscore.
122-
@return {String} the underscored string.
123-
@private
124-
*/
125-
export function underscore(str) {
126-
return UNDERSCORE_CACHE.get(str);
127-
}

packages/@ember/engine/tests/resolver/basic-test.js

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,4 +355,87 @@ module('strict-resolver | basic', function (hooks) {
355355

356356
assert.strictEqual(result, 'whatever', 'super-duper-config/environment is found');
357357
});
358+
359+
test('default plural handles -s / -ss / -sh / -ch / -x / -z suffixes', function (assert) {
360+
let cases = {
361+
'./buses/red': 'bus:red',
362+
'./brushes/broom': 'brush:broom',
363+
'./benches/park': 'bench:park',
364+
'./boxes/cardboard': 'box:cardboard',
365+
'./buzzes/loud': 'buzz:loud',
366+
'./classes/math': 'class:math',
367+
};
368+
369+
for (let [modulePath, lookup] of Object.entries(cases)) {
370+
let r = new StrictResolver({ [modulePath]: modulePath });
371+
assert.strictEqual(r.resolve(lookup), modulePath, `${lookup} -> ${modulePath}`);
372+
}
373+
});
374+
375+
test('default plural handles consonant + y suffix (y -> ies)', function (assert) {
376+
let r = new StrictResolver({ './categories/widgets': 'widgets-cat' });
377+
378+
assert.strictEqual(r.resolve('category:widgets'), 'widgets-cat');
379+
});
380+
381+
test('default plural handles common irregular nouns', function (assert) {
382+
let cases = {
383+
'./children/alice': 'child:alice',
384+
'./people/bob': 'person:bob',
385+
'./men/carl': 'man:carl',
386+
'./women/dana': 'woman:dana',
387+
'./mice/squeaky': 'mouse:squeaky',
388+
'./teeth/molar': 'tooth:molar',
389+
'./feet/left': 'foot:left',
390+
};
391+
392+
for (let [modulePath, lookup] of Object.entries(cases)) {
393+
let r = new StrictResolver({ [modulePath]: modulePath });
394+
assert.strictEqual(r.resolve(lookup), modulePath, `${lookup} -> ${modulePath}`);
395+
}
396+
});
397+
398+
test('custom plural overrides irregular default', function (assert) {
399+
// a user who insists on "childs" should be able to opt out of the
400+
// built-in irregular plural
401+
let r = new StrictResolver({ './childs/alice': 'alice' }, { child: 'childs' });
402+
403+
assert.strictEqual(r.resolve('child:alice'), 'alice');
404+
});
405+
406+
test('can lookup a nested-colocation component (index file)', function (assert) {
407+
let expected = { isComponentFactory: true };
408+
resolver.addModules({
409+
'./components/my-widget/index': { default: expected },
410+
});
411+
412+
assert.strictEqual(resolver.resolve('component:my-widget'), expected);
413+
});
414+
415+
test('nested-colocation also works for helpers and modifiers', function (assert) {
416+
let helper = {};
417+
let modifier = {};
418+
resolver.addModules({
419+
'./helpers/format-date/index': { default: helper },
420+
'./modifiers/on-intersect/index': { default: modifier },
421+
});
422+
423+
assert.strictEqual(resolver.resolve('helper:format-date'), helper);
424+
assert.strictEqual(resolver.resolve('modifier:on-intersect'), modifier);
425+
});
426+
427+
test('direct module takes precedence over the nested-colocation index', function (assert) {
428+
let direct = { direct: true };
429+
let nested = { nested: true };
430+
resolver.addModules({
431+
'./components/my-widget': { default: direct },
432+
'./components/my-widget/index': { default: nested },
433+
});
434+
435+
assert.strictEqual(
436+
resolver.resolve('component:my-widget'),
437+
direct,
438+
'direct match wins over the colocation fallback'
439+
);
440+
});
358441
});

packages/@ember/engine/tests/resolver/classify_test.js

Lines changed: 0 additions & 64 deletions
This file was deleted.

packages/@ember/engine/tests/resolver/decamelize_test.js

Lines changed: 0 additions & 32 deletions
This file was deleted.

0 commit comments

Comments
 (0)