From 2b945e10a585fab047f13777f8d78e50641328d3 Mon Sep 17 00:00:00 2001 From: "Gabriel J. Csapo" Date: Mon, 19 Dec 2022 08:47:14 -0800 Subject: [PATCH 1/9] feat: adds the strict resolver as an option --- README.md | 105 ++++++- addon/addon/resolvers/strict/index.js | 130 ++++++++ package-lock.json | 10 +- .../tests/unit/resolvers/strict/basic-test.js | 288 ++++++++++++++++++ 4 files changed, 521 insertions(+), 12 deletions(-) create mode 100644 addon/addon/resolvers/strict/index.js create mode 100644 test-app/tests/unit/resolvers/strict/basic-test.js diff --git a/README.md b/README.md index 83d9326d..e007d3dd 100644 --- a/README.md +++ b/README.md @@ -36,26 +36,115 @@ export default class App extends Application { // ...snip... ``` +## Strict + +> Originally from + +in app/resolver.js + +```js +export { default } from "ember-strict-resolver"; +``` + +_For additional improvements when fully using the ember-strict-resolver monkey patching the registry to no longer cache and simply returning the values passed like the following can be produce extra performance._ + +```js +// disable the normalization cache as we no longer normalize, the cache has become a bottle neck. +Ember.Registry.prototype.normalize = function (i) { + return i; +}; +``` + +## Migration + +Migrating away from use the _ember-resolver/classic_ can be done in piecemeal by supporting a sub-set of the old resolution formats. + +> normalize is needed, because without it you will get errors related to failing to be able to inject services that were never normalized in the registry. + +```js +// app/resolver.js + +import Resolver from "ember-strict-resolver"; + +export default class extends Resolver { + legacyMappings = { + "service:camelCaseNotSupported": "service:camel-case-not-supported", + }; + + resolve(_fullName) { + return super.resolve(this.legacyMappings[_fullName] || _fullName); + } + + normalize(_fullName) { + return this.legacyMappings[_fullName] || _fullName; + } +} +``` + +This will allow you file PRs with libraries that currently do not support the strict resolver in its entirety. + +In the event that you have a component that is failing to resolve correctly with the error `Attempted to lookup "helper:nameOfVariable". Use "helper:name-of-variable" instead.` please convert your template to use explicit-this. The template lint can be enabled by turning on [no-implicit-this](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-implicit-this.md). + +An example of what this looks like is the following + +```hbs +// addon/components/templates/foo.hbs + +
+ {{fullName}} +
+``` + +This will result in the error, `Attempted to lookup "helper:fullName". Use "helper:full-name" instead.`. The fix for this would be to decide if this is a argument being passed into foo or if this is a local property. + +_fullName_ is coming from an invocation of _Foo_ like the following: + +``` + +``` + +Then the fix for your template would be: + +```hbs +// addon/components/templates/foo.hbs + +
+ {{@fullName}} +
+``` + +If _fullName_ is a property on your component the fix would be: + +```hbs +// addon/components/templates/foo.hbs + +
+ {{this.fullName}} +
+``` + ## Addon Development ### Installation -* `git clone` this repository -* `npm install` -* `bower install` +- `git clone` this repository +- `npm install` +- `bower install` ### Running -* `ember server` -* Visit your app at http://localhost:4200. +- `ember server` +- Visit your app at http://localhost:4200. ### Running Tests -* `ember test` -* `ember test --server` +- `ember test` +- `ember test --server` ### Building -* `ember build` +- `ember build` For more information on using ember-cli, visit [http://www.ember-cli.com/](http://www.ember-cli.com/). diff --git a/addon/addon/resolvers/strict/index.js b/addon/addon/resolvers/strict/index.js new file mode 100644 index 00000000..abac2f49 --- /dev/null +++ b/addon/addon/resolvers/strict/index.js @@ -0,0 +1,130 @@ +/* globals requirejs */ + +import { warn } from '@ember/debug'; +import { dasherize } from '@ember/string'; +import { DEBUG } from '@glimmer/env'; + +import require from 'require'; + +export default class Resolver { + constructor(attrs) { + if (attrs) { + this.namespace = attrs.namespace; + } + // secret handshake with router to ensure substates are enabled + // see https://github.com/emberjs/ember.js/blob/a429dc327ee6ef97d948a83e727886c75c6fe043/packages/%40ember/-internals/routing/lib/system/router.ts#L344 + this.moduleBasedResolver = true; + } + + static create(args) { + return new this(args); + } + + has(moduleName) { + return moduleName in (requirejs.entries || requirejs._eak_seen); + } + + parseFullName(fullName) { + let prefix, type, name; + + let fullNameParts = fullName.split('@'); + + if (fullNameParts.length === 3) { + if (fullNameParts[0].length === 0) { + // leading scoped namespace: `@scope/pkg@type:name` + prefix = `@${fullNameParts[1]}`; + let prefixParts = fullNameParts[2].split(':'); + type = prefixParts[0]; + name = prefixParts[1]; + } else { + // interweaved scoped namespace: `type:@scope/pkg@name` + prefix = `@${fullNameParts[1]}`; + type = fullNameParts[0].slice(0, -1); + name = fullNameParts[2]; + } + + if (type === 'template:components') { + name = `components/${name}`; + type = 'template'; + } + } else if (fullNameParts.length === 2) { + let prefixParts = fullNameParts[0].split(':'); + + if (prefixParts.length === 2) { + if (prefixParts[1].length === 0) { + type = prefixParts[0]; + name = `@${fullNameParts[1]}`; + } else { + prefix = prefixParts[1]; + type = prefixParts[0]; + name = fullNameParts[1]; + } + } else { + let nameParts = fullNameParts[1].split(':'); + + prefix = fullNameParts[0]; + type = nameParts[0]; + name = nameParts[1]; + } + + if (type === 'template' && prefix.lastIndexOf('components/', 0) === 0) { + name = `components/${name}`; + prefix = prefix.slice(11); + } + } else { + fullNameParts = fullName.split(':'); + + prefix = this.namespace.modulePrefix; + type = fullNameParts[0]; + name = fullNameParts[1]; + } + + return { + prefix, + type, + name + } + } + + moduleNameForFullName(fullName) { + let moduleName; + + const { prefix, type, name } = this.parseFullName(fullName); + + if (name === 'main') { + moduleName = `${prefix}/${type}`; + } else if (type === 'engine') { + moduleName = `${name}/engine`; + } else if (type === 'route-map') { + moduleName = `${name}/routes`; + } else if (type === 'config') { + moduleName = `${prefix}/${type}/${name.replace(/\./g, '/')}`; + } else { + moduleName = `${prefix}/${type}s/${name.replace(/\./g, '/')}`; + } + + return moduleName; + } + + resolve(fullName) { + const moduleName = this.moduleNameForFullName(fullName); + + if (this.has(moduleName)) { + // hit + return require(moduleName)['default']; + } + // miss + } + + normalize(fullName) { + if(DEBUG) { + const { type } = this.parseFullName(fullName); + + if(['service'].includes(type)) { + warn(`Attempted to lookup "${fullName}". Use "${dasherize(fullName)}" instead.`, !fullName.match(/[a-z]+[A-Z]+/), { id: 'ember-strict-resolver.camelcase-names' }); + } + } + + return fullName; + } +} diff --git a/package-lock.json b/package-lock.json index 01d86af9..666d8e2e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,9 +1,11 @@ { "name": "ember-resolver", + "version": "9.0.0", "lockfileVersion": 2, "requires": true, "packages": { "": { + "version": "9.0.0", "workspaces": [ "addon", "test-app" @@ -24,7 +26,7 @@ }, "addon": { "name": "ember-resolver", - "version": "8.0.3", + "version": "9.0.0", "license": "MIT", "dependencies": { "ember-cli-babel": "^7.26.11" @@ -26678,7 +26680,7 @@ } }, "test-app": { - "version": "8.0.3", + "version": "9.0.0", "license": "MIT", "devDependencies": { "@ember/optional-features": "^2.0.0", @@ -26694,7 +26696,7 @@ "ember-disable-prototype-extensions": "^1.1.3", "ember-load-initializers": "^2.1.2", "ember-qunit": "^5.1.5", - "ember-resolver": "8.0.3", + "ember-resolver": "9.0.0", "ember-source": "~4.9.3", "ember-source-channel-url": "^3.0.0", "ember-try": "^2.0.0", @@ -45979,7 +45981,7 @@ "ember-disable-prototype-extensions": "^1.1.3", "ember-load-initializers": "^2.1.2", "ember-qunit": "^5.1.5", - "ember-resolver": "8.0.3", + "ember-resolver": "9.0.0", "ember-source": "~4.9.3", "ember-source-channel-url": "^3.0.0", "ember-try": "^2.0.0", diff --git a/test-app/tests/unit/resolvers/strict/basic-test.js b/test-app/tests/unit/resolvers/strict/basic-test.js new file mode 100644 index 00000000..68e59290 --- /dev/null +++ b/test-app/tests/unit/resolvers/strict/basic-test.js @@ -0,0 +1,288 @@ +/* globals define, requirejs */ + +import { assign } from "@ember/polyfills"; +import { module, test } from "qunit"; +import { setupTest } from "ember-qunit"; +import Resolver from "ember-resolver/resolvers/strict"; + +let originalRegistryEntries, resolver; + +function setupResolver(options) { + if (!options) { + options = { namespace: { modulePrefix: "foo-bar" } }; + } + + resolver = Resolver.create(options); +} + +function resetRegistry() { + requirejs.clear(); + assign(requirejs.entries, originalRegistryEntries); +} + +module("ember-resolver/resolvers/strict", function (hooks) { + setupTest(hooks); + + hooks.beforeEach(function () { + originalRegistryEntries = assign({}, requirejs.entries); + + setupResolver(); + }); + + hooks.afterEach(function () { + resetRegistry(); + }); + + test("does not require `namespace` to exist at `init` time", function (assert) { + assert.expect(0); + + Resolver.create(); + }); + + test("is module based", function (assert) { + assert.strictEqual( + resolver.moduleBasedResolver, + true, + "resolver declares itself module-based to enable router features" + ); + }); + + test("moduleNameForFullName", function (assert) { + const testPairs = [ + ["route:application", "foo-bar/routes/application"], + ["route:application/index", "foo-bar/routes/application/index"], + ["application:main", "foo-bar/application"], + ["route:foo.bar.baz.index", "foo-bar/routes/foo/bar/baz/index"], + ["config:environment", "foo-bar/config/environment"], + ]; + + assert.expect(testPairs.length); + + testPairs.forEach((pair) => { + assert.equal( + resolver.moduleNameForFullName(pair[0]), + pair[1], + `fullName was not resolved correctly for ${pair[0]}` + ); + }); + }); + + test("can lookup something", function (assert) { + assert.expect(2); + + define("foo-bar/adapters/post", [], function () { + assert.ok(true, "adapter was invoked properly"); + + return {}; + }); + + var adapter = resolver.resolve("adapter:post"); + + assert.ok(adapter, "adapter was returned"); + }); + + test("can lookup something in another namespace", function (assert) { + assert.expect(3); + + let expected = {}; + + define("other/adapters/post", [], function () { + assert.ok(true, "adapter was invoked properly"); + + return { + default: expected, + }; + }); + + var adapter = resolver.resolve("other@adapter:post"); + + assert.ok(adapter, "adapter was returned"); + assert.equal(adapter, expected, "default export was returned"); + }); + + test("can lookup something in another namespace with different syntax", function (assert) { + assert.expect(3); + + let expected = {}; + define("other/adapters/post", [], function () { + assert.ok(true, "adapter was invoked properly"); + + return { default: expected }; + }); + + var adapter = resolver.resolve("adapter:other@post"); + + assert.ok(adapter, "adapter was returned"); + assert.equal(adapter, expected, "default export was returned"); + }); + + test("can lookup a helper", function (assert) { + assert.expect(3); + + let expected = { isHelperInstance: true }; + define("foo-bar/helpers/reverse-list", [], function () { + assert.ok(true, "helper was invoked properly"); + + return { default: expected }; + }); + + var helper = resolver.resolve("helper:reverse-list"); + + assert.ok(helper, "helper was returned"); + assert.equal(helper, expected, "default export was returned"); + }); + + test("can lookup an engine", function (assert) { + assert.expect(3); + + let expected = {}; + define("foo-bar/engine", [], function () { + assert.ok(true, "engine was invoked properly"); + + return { default: expected }; + }); + + let engine = resolver.resolve("engine:foo-bar"); + + assert.ok(engine, "engine was returned"); + assert.equal(engine, expected, "default export was returned"); + }); + + test("can lookup a route-map", function (assert) { + assert.expect(3); + + let expected = { isRouteMap: true }; + define("foo-bar/routes", [], function () { + assert.ok(true, "route-map was invoked properly"); + + return { default: expected }; + }); + + let routeMap = resolver.resolve("route-map:foo-bar"); + + assert.ok(routeMap, "route-map was returned"); + assert.equal(routeMap, expected, "default export was returned"); + }); + + test("warns if normalizations up a camelCase service name", function (assert) { + assert.expect(1); + + define("foo-bar/services/reverse-list", [], function () { + return { default: { isHelperInstance: true } }; + }); + + assert.equal( + "service:reverseList", + resolver.normalize("service:reverseList"), + "no service was returned" + ); + // assert.expectWarning( + // 'Attempted to lookup "service:reverseList". Use "service:reverse-list" instead.' + // ); + }); + + test("will unwrap the 'default' export automatically", function (assert) { + define("foo-bar/fruits/orange", [], function () { + return { default: "is awesome" }; + }); + + assert.equal( + resolver.resolve("fruit:orange"), + "is awesome", + "adapter was returned" + ); + }); + + test("router:main is hard-coded to prefix/router.js", function (assert) { + assert.expect(1); + + define("foo-bar/router", [], function () { + assert.ok(true, "router:main was looked up"); + return "whatever"; + }); + + resolver.resolve("router:main"); + }); + + test("store:main is looked up as prefix/store", function (assert) { + assert.expect(1); + + define("foo-bar/store", [], function () { + assert.ok(true, "store:main was looked up"); + return "whatever"; + }); + + resolver.resolve("store:main"); + }); + + test("store:posts as prefix/stores/post", function (assert) { + assert.expect(1); + + define("foo-bar/stores/post", [], function () { + assert.ok(true, "store:post was looked up"); + return "whatever"; + }); + + resolver.resolve("store:post"); + }); + + test("can lookup a component template in another namespace with different syntax", function (assert) { + assert.expect(2); + + let expected = { isTemplate: true }; + define("other/templates/components/foo-bar", [], function () { + assert.ok(true, "template was looked up properly"); + + return { default: expected }; + }); + + var template = resolver.resolve("template:components/other@foo-bar"); + + assert.equal(template, expected, "default export was returned"); + }); + + test("does not lookup index when top level component is specified (route:view)", function (assert) { + assert.expect(1); + + define("foo-bar/routes/view/index", [], function () { + assert.ok(false, "should not have been required"); + }); + + assert.ok(!resolver.resolve("route:view"), "route was not returned"); + }); + + test("can lookup an engine from a scoped package", function (assert) { + assert.expect(3); + + let expected = {}; + define("@some-scope/some-module/engine", [], function () { + assert.ok(true, "engine was invoked properly"); + + return { default: expected }; + }); + + var engine = resolver.resolve("engine:@some-scope/some-module"); + + assert.ok(engine, "engine was returned"); + assert.equal(engine, expected, "default export was returned"); + }); + + test("can lookup something in another namespace with an @ scope", function (assert) { + assert.expect(3); + + let expected = {}; + + define("@scope/other/adapters/post", [], function () { + assert.ok(true, "adapter was invoked properly"); + + return { + default: expected, + }; + }); + + var adapter = resolver.resolve("@scope/other@adapter:post"); + + assert.ok(adapter, "adapter was returned"); + assert.equal(adapter, expected, "default export was returned"); + }); +}); From 73d85c2755be0b1ac7220425afe2b921c9060b2b Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:07 -0800 Subject: [PATCH 2/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e007d3dd..91aedd88 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ export default class App extends Application { ## Strict -> Originally from +> **Note** This was originally developed as in app/resolver.js From 7e412012c2628298fe1542e30cd4fdf28e2f7249 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:13 -0800 Subject: [PATCH 3/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 91aedd88..8445ce8e 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,7 @@ export default class App extends Application { in app/resolver.js ```js -export { default } from "ember-strict-resolver"; +export { default } from "ember-resolver/strict"; ``` _For additional improvements when fully using the ember-strict-resolver monkey patching the registry to no longer cache and simply returning the values passed like the following can be produce extra performance._ From eff25f8f46fc2c3f62018cb608e8d902c9e2e91f Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:20 -0800 Subject: [PATCH 4/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8445ce8e..597953c4 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ in app/resolver.js export { default } from "ember-resolver/strict"; ``` -_For additional improvements when fully using the ember-strict-resolver monkey patching the registry to no longer cache and simply returning the values passed like the following can be produce extra performance._ +_For additional improvements when fully using the strict resolver, monkey patching the registry to no longer cache and simply returning the values passed can be produce extra performance:_ ```js // disable the normalization cache as we no longer normalize, the cache has become a bottle neck. From 523dedd5752936abb7a81c4ecae35a97acef4261 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:27 -0800 Subject: [PATCH 5/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 597953c4..7c46989a 100644 --- a/README.md +++ b/README.md @@ -57,7 +57,7 @@ Ember.Registry.prototype.normalize = function (i) { ## Migration -Migrating away from use the _ember-resolver/classic_ can be done in piecemeal by supporting a sub-set of the old resolution formats. +Migrating to the `strict` resolver from the `classic` can be done piecemeal by supporting a sub-set of the old resolution formats. > normalize is needed, because without it you will get errors related to failing to be able to inject services that were never normalized in the registry. From 06f869d40d0c317c655928d8a5d45faf11fccc77 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:32 -0800 Subject: [PATCH 6/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7c46989a..f177fb49 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ Ember.Registry.prototype.normalize = function (i) { Migrating to the `strict` resolver from the `classic` can be done piecemeal by supporting a sub-set of the old resolution formats. -> normalize is needed, because without it you will get errors related to failing to be able to inject services that were never normalized in the registry. +> **Note** `normalize` is needed because without it you will get errors related to failing to be able to inject services that were never normalized in the registry. ```js // app/resolver.js From 514bfed13254d504d3c4af6ee7610c13bb2bb199 Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:44 -0800 Subject: [PATCH 7/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index f177fb49..b4ec4fa7 100644 --- a/README.md +++ b/README.md @@ -83,7 +83,7 @@ export default class extends Resolver { This will allow you file PRs with libraries that currently do not support the strict resolver in its entirety. -In the event that you have a component that is failing to resolve correctly with the error `Attempted to lookup "helper:nameOfVariable". Use "helper:name-of-variable" instead.` please convert your template to use explicit-this. The template lint can be enabled by turning on [no-implicit-this](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-implicit-this.md). +If you have a component that is failing to resolve correctly with the error `Attempted to lookup "helper:nameOfVariable". Use "helper:name-of-variable" instead.`, please convert your template to use explicit-this (also required by Ember v4.0). The template lint can be enabled by turning on [no-implicit-this](https://github.com/ember-template-lint/ember-template-lint/blob/master/docs/rule/no-implicit-this.md). An example of what this looks like is the following From 82d4fe90523ad91943027d2ca8a81ef7ca10555f Mon Sep 17 00:00:00 2001 From: Gabriel Csapo Date: Mon, 19 Dec 2022 09:03:50 -0800 Subject: [PATCH 8/9] Update README.md Co-authored-by: Chris Krycho --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index b4ec4fa7..19f9f2bd 100644 --- a/README.md +++ b/README.md @@ -64,7 +64,7 @@ Migrating to the `strict` resolver from the `classic` can be done piecemeal by s ```js // app/resolver.js -import Resolver from "ember-strict-resolver"; +import Resolver from "ember-resolver/strict"; export default class extends Resolver { legacyMappings = { From 7aa9efd331a5e755afda567eece0b357d2da2ebd Mon Sep 17 00:00:00 2001 From: "Gabriel J. Csapo" Date: Wed, 21 Dec 2022 09:29:23 -0800 Subject: [PATCH 9/9] chore: remove readme content about monkeypatching --- README.md | 9 --------- 1 file changed, 9 deletions(-) diff --git a/README.md b/README.md index 19f9f2bd..9e8faf51 100644 --- a/README.md +++ b/README.md @@ -46,15 +46,6 @@ in app/resolver.js export { default } from "ember-resolver/strict"; ``` -_For additional improvements when fully using the strict resolver, monkey patching the registry to no longer cache and simply returning the values passed can be produce extra performance:_ - -```js -// disable the normalization cache as we no longer normalize, the cache has become a bottle neck. -Ember.Registry.prototype.normalize = function (i) { - return i; -}; -``` - ## Migration Migrating to the `strict` resolver from the `classic` can be done piecemeal by supporting a sub-set of the old resolution formats.