From 7e372b3a159aed6be414486ec3ef0e259ea66cb5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Mon, 17 Oct 2022 09:33:52 +0100 Subject: [PATCH] DEV: Restrict resolver lookups to known namespaces (#18599) Ember's default resolver only looks for components/services/etc. which are namespaced under the app's `modulePrefix` (`discourse`, in our case). To use addon components/services/etc., the addon must re-export them in its `app/` directory. In order to support plugins, our custom resolver does a 'suffix match'. This has an unintended side-effect of matching things which are not part of the discourse app or themes/plugins. We've come to rely on this for a few in-repo addons like `select-kit`, `admin` and `wizard`. This unrestricted 'suffix matching' can cause some very unexpected behaviour. For example, the ember-inspector browser extension has a module called `ember_debug/service/session`. When looking up `service:session`, our resolver was choosing that third-party service over our own Session service. This means Discourse fails to boot when the Ember Inspector is open. This commit restricts the 'suffix matching' to a known set of namespaces. This brings us one step closer to the default Ember Resolver implementation, and reduces the chance of unexpected behaviour like the ember-inspector issue. This commit also updates the `dialog-holder` addon to export its service under the app directory, so that we don't need to account for it in the resolver. We may want to consider doing the same for things like `select-kit` and `truth-helpers`, but is beyond the scope of this commit. --- .../javascripts/discourse-common/addon/resolver.js | 13 ++++++++++++- .../lib/dialog-holder/app/services/dialog.js | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 app/assets/javascripts/discourse/lib/dialog-holder/app/services/dialog.js diff --git a/app/assets/javascripts/discourse-common/addon/resolver.js b/app/assets/javascripts/discourse-common/addon/resolver.js index 462bec5212d..7fc67738b05 100644 --- a/app/assets/javascripts/discourse-common/addon/resolver.js +++ b/app/assets/javascripts/discourse-common/addon/resolver.js @@ -119,8 +119,19 @@ export function clearResolverOptions() { function lookupModuleBySuffix(suffix) { if (!moduleSuffixTrie) { moduleSuffixTrie = new SuffixTrie("/"); + const searchPaths = [ + "discourse/", // Includes themes/plugins + "discourse-common/", + "select-kit/", + "admin/", + "wizard/", + "truth-helpers/", + ]; Object.keys(requirejs.entries).forEach((name) => { - if (!name.includes("/templates/")) { + if ( + searchPaths.some((s) => name.startsWith(s)) && + !name.includes("/templates/") + ) { moduleSuffixTrie.add(name); } }); diff --git a/app/assets/javascripts/discourse/lib/dialog-holder/app/services/dialog.js b/app/assets/javascripts/discourse/lib/dialog-holder/app/services/dialog.js new file mode 100644 index 00000000000..cf0c5483acd --- /dev/null +++ b/app/assets/javascripts/discourse/lib/dialog-holder/app/services/dialog.js @@ -0,0 +1 @@ +export { default } from "dialog-holder/services/dialog";