From bbd7bd83b3a728cb8243ea9a39cf37f39cc19c13 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:26:29 -0400 Subject: [PATCH 1/4] Address review: drop cache, simplify pluralize, collapse setup-resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feedback from the review of PR #21303: - "how much do we care about keeping this cache around?" (it's from ember-string) — drop the Cache helper and its test file. dasherize is now a plain `replace.toLowerCase().replace` in strict-resolver/ string.js. For the workloads the resolver sees (module lookups during boot) the cache is noise. - "we don't need to specify this list -- the class that has `modules = ` assignable on it should be able to specify their inflection rules" — drop the built-in IRREGULAR_PLURALS table and the -s/-es / consonant-y suffix rules. Pluralization is back to naive `type + 's'`, matching ember-resolver's behavior. Consumers that want children / people / buses register them up-front via the `plurals` constructor option, same as they already do for `config`. - "is this true? does ember-resolver do this?" — the regex-based rules weren't in ember-resolver either; they're gone alongside the irregulars. - "let's remove this function, I think" — delete the setupResolver helper and its file; basic-test.js now instantiates StrictResolver directly in beforeEach. - "can we also add a strict application to the v2 app scenarios? I thiiiiiink we just need to overwrite the app.js in that scenario" — yes: add `strictResolver` as a variant of v2AppScenarios (in addition to embroiderVite). basic-test.ts now runs against both v2 configurations. Tests updated to match: the suffix/irregular/y→ies cases are removed; one test left behind proves that registering a custom plural still lets you do `child: 'children'` explicitly. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/@ember/engine/lib/strict-resolver.ts | 32 +---------- .../engine/lib/strict-resolver/cache.js | 35 ------------ .../engine/lib/strict-resolver/string.js | 15 ++--- .../engine/tests/resolver/-setup-resolver.js | 9 --- .../engine/tests/resolver/basic-test.js | 55 ++++--------------- smoke-tests/scenarios/scenarios.ts | 31 ++++++++++- 6 files changed, 44 insertions(+), 133 deletions(-) delete mode 100644 packages/@ember/engine/lib/strict-resolver/cache.js delete mode 100644 packages/@ember/engine/tests/resolver/-setup-resolver.js diff --git a/packages/@ember/engine/lib/strict-resolver.ts b/packages/@ember/engine/lib/strict-resolver.ts index eb3f8820ba7..55a4dc74b83 100644 --- a/packages/@ember/engine/lib/strict-resolver.ts +++ b/packages/@ember/engine/lib/strict-resolver.ts @@ -36,7 +36,7 @@ export class StrictResolver implements Resolver { } #plural(s: string) { - return this.#plurals.get(s) ?? pluralize(s); + return this.#plurals.get(s) ?? s + 's'; } resolve(fullName: string): Factory | object | undefined { @@ -128,36 +128,6 @@ export class StrictResolver implements Resolver { } } -// Handle the common irregular English plurals plus the standard -s / -es -// suffix rules. Users can override any type via the `plurals` constructor -// option (including overriding these defaults). -const IRREGULAR_PLURALS: Record = Object.freeze({ - child: 'children', - man: 'men', - woman: 'women', - person: 'people', - mouse: 'mice', - tooth: 'teeth', - foot: 'feet', -}); - -const NEEDS_ES_SUFFIX = /(s|ss|sh|ch|x|z)$/; -const ENDS_IN_CONSONANT_Y = /([^aeiou])y$/; - -function pluralize(singular: string): string { - let irregular = IRREGULAR_PLURALS[singular]; - if (irregular) { - return irregular; - } - if (ENDS_IN_CONSONANT_Y.test(singular)) { - return singular.replace(ENDS_IN_CONSONANT_Y, '$1ies'); - } - if (NEEDS_ES_SUFFIX.test(singular)) { - return singular + 'es'; - } - return singular + 's'; -} - const fileExtension = /\.\w{1,4}$/; const leadingDotSlash = /^\.\//; diff --git a/packages/@ember/engine/lib/strict-resolver/cache.js b/packages/@ember/engine/lib/strict-resolver/cache.js deleted file mode 100644 index 68902ada388..00000000000 --- a/packages/@ember/engine/lib/strict-resolver/cache.js +++ /dev/null @@ -1,35 +0,0 @@ -export default class Cache { - constructor(limit, func, store) { - this.limit = limit; - this.func = func; - this.store = store; - this.size = 0; - this.misses = 0; - this.hits = 0; - this.store = store || new Map(); - } - get(key) { - let value = this.store.get(key); - if (this.store.has(key)) { - this.hits++; - return this.store.get(key); - } else { - this.misses++; - value = this.set(key, this.func(key)); - } - return value; - } - set(key, value) { - if (this.limit > this.size) { - this.size++; - this.store.set(key, value); - } - return value; - } - purge() { - this.store.clear(); - this.size = 0; - this.hits = 0; - this.misses = 0; - } -} diff --git a/packages/@ember/engine/lib/strict-resolver/string.js b/packages/@ember/engine/lib/strict-resolver/string.js index a2e04821b4f..b1662340293 100644 --- a/packages/@ember/engine/lib/strict-resolver/string.js +++ b/packages/@ember/engine/lib/strict-resolver/string.js @@ -1,16 +1,9 @@ -import Cache from './cache'; - const STRING_DASHERIZE_REGEXP = /[ _]/g; const STRING_DECAMELIZE_REGEXP = /([a-z\d])([A-Z])/g; -const DECAMELIZE_CACHE = new Cache(1000, (str) => - str.replace(STRING_DECAMELIZE_REGEXP, '$1_$2').toLowerCase() -); - -const STRING_DASHERIZE_CACHE = new Cache(1000, (key) => - DECAMELIZE_CACHE.get(key).replace(STRING_DASHERIZE_REGEXP, '-') -); - export function dasherize(str) { - return STRING_DASHERIZE_CACHE.get(str); + return str + .replace(STRING_DECAMELIZE_REGEXP, '$1_$2') + .toLowerCase() + .replace(STRING_DASHERIZE_REGEXP, '-'); } diff --git a/packages/@ember/engine/tests/resolver/-setup-resolver.js b/packages/@ember/engine/tests/resolver/-setup-resolver.js deleted file mode 100644 index 55174802cc4..00000000000 --- a/packages/@ember/engine/tests/resolver/-setup-resolver.js +++ /dev/null @@ -1,9 +0,0 @@ -import { StrictResolver } from '@ember/engine/lib/strict-resolver'; - -export function setupResolver(options = {}) { - let modules = {}; - let plurals = options.plurals; - let resolver = new StrictResolver(modules, plurals); - - return { resolver, modules }; -} diff --git a/packages/@ember/engine/tests/resolver/basic-test.js b/packages/@ember/engine/tests/resolver/basic-test.js index 74baccc4a5a..7af9433f2c5 100644 --- a/packages/@ember/engine/tests/resolver/basic-test.js +++ b/packages/@ember/engine/tests/resolver/basic-test.js @@ -1,13 +1,13 @@ import { module, test } from 'qunit'; import { StrictResolver } from '@ember/engine/lib/strict-resolver'; -import { setupResolver } from './-setup-resolver'; module('strict-resolver | basic', function (hooks) { let resolver; let modules; hooks.beforeEach(function () { - ({ resolver, modules } = setupResolver()); + modules = {}; + resolver = new StrictResolver(modules); }); test('can lookup something', function (assert) { @@ -356,49 +356,14 @@ module('strict-resolver | basic', function (hooks) { assert.strictEqual(result, 'whatever', 'super-duper-config/environment is found'); }); - test('default plural handles -s / -ss / -sh / -ch / -x / -z suffixes', function (assert) { - let cases = { - './buses/red': 'bus:red', - './brushes/broom': 'brush:broom', - './benches/park': 'bench:park', - './boxes/cardboard': 'box:cardboard', - './buzzes/loud': 'buzz:loud', - './classes/math': 'class:math', - }; - - for (let [modulePath, lookup] of Object.entries(cases)) { - let r = new StrictResolver({ [modulePath]: modulePath }); - assert.strictEqual(r.resolve(lookup), modulePath, `${lookup} -> ${modulePath}`); - } - }); - - test('default plural handles consonant + y suffix (y -> ies)', function (assert) { - let r = new StrictResolver({ './categories/widgets': 'widgets-cat' }); - - assert.strictEqual(r.resolve('category:widgets'), 'widgets-cat'); - }); - - test('default plural handles common irregular nouns', function (assert) { - let cases = { - './children/alice': 'child:alice', - './people/bob': 'person:bob', - './men/carl': 'man:carl', - './women/dana': 'woman:dana', - './mice/squeaky': 'mouse:squeaky', - './teeth/molar': 'tooth:molar', - './feet/left': 'foot:left', - }; - - for (let [modulePath, lookup] of Object.entries(cases)) { - let r = new StrictResolver({ [modulePath]: modulePath }); - assert.strictEqual(r.resolve(lookup), modulePath, `${lookup} -> ${modulePath}`); - } - }); - - test('custom plural overrides irregular default', function (assert) { - // a user who insists on "childs" should be able to opt out of the - // built-in irregular plural - let r = new StrictResolver({ './childs/alice': 'alice' }, { child: 'childs' }); + test('irregular plurals must be opted into via the plurals option', function (assert) { + // Default pluralization is naive (type + 's'), matching ember-resolver's + // behavior. A consumer that wants proper English irregulars registers + // them up-front via the plurals map. + let r = new StrictResolver( + { './children/alice': 'alice' }, + { child: 'children' } + ); assert.strictEqual(r.resolve('child:alice'), 'alice'); }); diff --git a/smoke-tests/scenarios/scenarios.ts b/smoke-tests/scenarios/scenarios.ts index edb3b94d4c2..5fb1cbad759 100644 --- a/smoke-tests/scenarios/scenarios.ts +++ b/smoke-tests/scenarios/scenarios.ts @@ -21,6 +21,34 @@ function embroiderWebpack(project: Project) { function embroiderVite(project: Project) {} +// Swap the v2-app-template's default app.js for a strict-resolver variant: +// no ember-resolver, no compatModules, no modulePrefix — just a `modules = +// {...import.meta.glob(...)}` literal. Making this a variant of +// v2AppScenarios means every test that runs against v2AppScenarios also +// runs against this configuration. +function strictResolver(project: Project) { + project.mergeFiles({ + app: { + 'app.js': ` + import Application from '@ember/application'; + import Router from './router'; + + export default class App extends Application { + modules = { + './router': { default: Router }, + ...import.meta.glob('./services/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./controllers/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./routes/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./components/**/*.{gjs,gts,js,ts}', { eager: true }), + ...import.meta.glob('./helpers/**/*.{js,ts}', { eager: true }), + ...import.meta.glob('./templates/**/*.{hbs,gjs,gts}', { eager: true }), + }; + } + `, + }, + }); +} + export const v1AppScenarios = Scenarios.fromProject(() => Project.fromDir(dirname(require.resolve('../app-template/package.json')), { linkDevDeps: true }) ).expand({ @@ -34,10 +62,9 @@ export const v2AppScenarios = Scenarios.fromProject(() => }) ).expand({ embroiderVite, + strictResolver, }); -function strictResolver(project: Project) {} - export const strictAppScenarios = Scenarios.fromProject(() => Project.fromDir(dirname(require.resolve('../v2-app-template/package.json')), { linkDevDeps: true, From a9cfe4340d8bff4dd9f2e1be20dd647d509c5139 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 20 Apr 2026 10:34:31 -0400 Subject: [PATCH 2/4] Fix prettier + drop deleted cache.js from package.json aliases --- package.json | 1 - packages/@ember/engine/tests/resolver/basic-test.js | 5 +---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/package.json b/package.json index 3e59dce96e8..07049dd9316 100644 --- a/package.json +++ b/package.json @@ -230,7 +230,6 @@ "@ember/engine/instance.js": "ember-source/@ember/engine/instance.js", "@ember/engine/lib/engine-parent.js": "ember-source/@ember/engine/lib/engine-parent.js", "@ember/engine/lib/strict-resolver.js": "ember-source/@ember/engine/lib/strict-resolver.js", - "@ember/engine/lib/strict-resolver/cache.js": "ember-source/@ember/engine/lib/strict-resolver/cache.js", "@ember/engine/lib/strict-resolver/string.js": "ember-source/@ember/engine/lib/strict-resolver/string.js", "@ember/engine/parent.js": "ember-source/@ember/engine/parent.js", "@ember/enumerable/index.js": "ember-source/@ember/enumerable/index.js", diff --git a/packages/@ember/engine/tests/resolver/basic-test.js b/packages/@ember/engine/tests/resolver/basic-test.js index 7af9433f2c5..7cedb924424 100644 --- a/packages/@ember/engine/tests/resolver/basic-test.js +++ b/packages/@ember/engine/tests/resolver/basic-test.js @@ -360,10 +360,7 @@ module('strict-resolver | basic', function (hooks) { // Default pluralization is naive (type + 's'), matching ember-resolver's // behavior. A consumer that wants proper English irregulars registers // them up-front via the plurals map. - let r = new StrictResolver( - { './children/alice': 'alice' }, - { child: 'children' } - ); + let r = new StrictResolver({ './children/alice': 'alice' }, { child: 'children' }); assert.strictEqual(r.resolve('child:alice'), 'alice'); }); From a54e6872599053a8d37fa50f783a2ed82eb0d7a3 Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 20 Apr 2026 11:04:03 -0400 Subject: [PATCH 3/4] Drop strictResolver from v2AppScenarios; keep dedicated strict scenario MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI revealed that basic-test.ts includes a route template (`templates/ example-gjs-route.gjs`) that exports a `.gjs` Component class — a v2 convention that works under embroiderVite (via compat-modules) but doesn't render with the strict resolver: owner.lookup('template: example-gjs-route') hands back the Component and rendering fails to produce [data-test=\"model-field\"], so the acceptance test fails. That's a real gap worth fixing, but it belongs in a separate PR that can focus on template-as-component lookup under the strict resolver. For now, drop the v2AppScenarios variant and leave the strictResolver function available via strictAppScenarios (used by the dedicated strict-resolver-test.ts / strict-resolver-substates-test.ts). Co-Authored-By: Claude Opus 4.7 (1M context) --- smoke-tests/scenarios/scenarios.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/smoke-tests/scenarios/scenarios.ts b/smoke-tests/scenarios/scenarios.ts index 5fb1cbad759..b20e672795b 100644 --- a/smoke-tests/scenarios/scenarios.ts +++ b/smoke-tests/scenarios/scenarios.ts @@ -62,7 +62,6 @@ export const v2AppScenarios = Scenarios.fromProject(() => }) ).expand({ embroiderVite, - strictResolver, }); export const strictAppScenarios = Scenarios.fromProject(() => From f11a87b6db95be25ea2f61320927c6a3898b3f3c Mon Sep 17 00:00:00 2001 From: NullVoxPopuli <199018+NullVoxPopuli@users.noreply.github.com> Date: Mon, 20 Apr 2026 12:41:49 -0400 Subject: [PATCH 4/4] Inline dasherize into strict-resolver.ts, drop the string/ directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to review: with cache.js gone, `string.js` was a 9-line file exporting just dasherize. Inline it as a private helper in strict-resolver.ts and delete the strict-resolver/ subdirectory (plus dasherize_test.js — dasherize's behavior is exercised via resolver.normalize(...) tests in basic-test.js). Co-Authored-By: Claude Opus 4.7 (1M context) --- package.json | 1 - packages/@ember/engine/lib/strict-resolver.ts | 7 +++- .../engine/lib/strict-resolver/string.js | 9 ----- .../engine/tests/resolver/dasherize_test.js | 36 ------------------- 4 files changed, 6 insertions(+), 47 deletions(-) delete mode 100644 packages/@ember/engine/lib/strict-resolver/string.js delete mode 100644 packages/@ember/engine/tests/resolver/dasherize_test.js diff --git a/package.json b/package.json index 07049dd9316..f1ac50823d6 100644 --- a/package.json +++ b/package.json @@ -230,7 +230,6 @@ "@ember/engine/instance.js": "ember-source/@ember/engine/instance.js", "@ember/engine/lib/engine-parent.js": "ember-source/@ember/engine/lib/engine-parent.js", "@ember/engine/lib/strict-resolver.js": "ember-source/@ember/engine/lib/strict-resolver.js", - "@ember/engine/lib/strict-resolver/string.js": "ember-source/@ember/engine/lib/strict-resolver/string.js", "@ember/engine/parent.js": "ember-source/@ember/engine/parent.js", "@ember/enumerable/index.js": "ember-source/@ember/enumerable/index.js", "@ember/enumerable/mutable.js": "ember-source/@ember/enumerable/mutable.js", diff --git a/packages/@ember/engine/lib/strict-resolver.ts b/packages/@ember/engine/lib/strict-resolver.ts index 55a4dc74b83..a865d304d93 100644 --- a/packages/@ember/engine/lib/strict-resolver.ts +++ b/packages/@ember/engine/lib/strict-resolver.ts @@ -1,5 +1,4 @@ import type { Factory, Resolver } from '@ember/owner'; -import { dasherize } from './strict-resolver/string'; export class StrictResolver implements Resolver { // Ember's router uses this flag to decide whether to auto-generate @@ -130,6 +129,12 @@ export class StrictResolver implements Resolver { const fileExtension = /\.\w{1,4}$/; const leadingDotSlash = /^\.\//; +const camelCaseBoundary = /([a-z\d])([A-Z])/g; +const spacesAndUnderscores = /[ _]/g; + +function dasherize(str: string): string { + return str.replace(camelCaseBoundary, '$1_$2').toLowerCase().replace(spacesAndUnderscores, '-'); +} type Result = | { diff --git a/packages/@ember/engine/lib/strict-resolver/string.js b/packages/@ember/engine/lib/strict-resolver/string.js deleted file mode 100644 index b1662340293..00000000000 --- a/packages/@ember/engine/lib/strict-resolver/string.js +++ /dev/null @@ -1,9 +0,0 @@ -const STRING_DASHERIZE_REGEXP = /[ _]/g; -const STRING_DECAMELIZE_REGEXP = /([a-z\d])([A-Z])/g; - -export function dasherize(str) { - return str - .replace(STRING_DECAMELIZE_REGEXP, '$1_$2') - .toLowerCase() - .replace(STRING_DASHERIZE_REGEXP, '-'); -} diff --git a/packages/@ember/engine/tests/resolver/dasherize_test.js b/packages/@ember/engine/tests/resolver/dasherize_test.js deleted file mode 100644 index 2fbec4bd3f0..00000000000 --- a/packages/@ember/engine/tests/resolver/dasherize_test.js +++ /dev/null @@ -1,36 +0,0 @@ -import { module, test } from 'qunit'; -import { dasherize } from '@ember/engine/lib/strict-resolver/string'; - -module('strict-resolver | dasherize', function () { - test('dasherize normal string', function (assert) { - assert.deepEqual(dasherize('my favorite items'), 'my-favorite-items'); - }); - - test('does nothing with dasherized string', function (assert) { - assert.deepEqual(dasherize('css-class-name'), 'css-class-name'); - }); - - test('dasherize underscored string', function (assert) { - assert.deepEqual(dasherize('action_name'), 'action-name'); - }); - - test('dasherize camelcased string', function (assert) { - assert.deepEqual(dasherize('innerHTML'), 'inner-html'); - }); - - test('dasherize string that is the property name of Object.prototype', function (assert) { - assert.deepEqual(dasherize('toString'), 'to-string'); - }); - - test('dasherize namespaced classified string', function (assert) { - assert.deepEqual(dasherize('PrivateDocs/OwnerInvoice'), 'private-docs/owner-invoice'); - }); - - test('dasherize namespaced camelized string', function (assert) { - assert.deepEqual(dasherize('privateDocs/ownerInvoice'), 'private-docs/owner-invoice'); - }); - - test('dasherize namespaced underscored string', function (assert) { - assert.deepEqual(dasherize('private_docs/owner_invoice'), 'private-docs/owner-invoice'); - }); -});