DEV: Promote modifyClass warning to error (#28300)

This message indicates broken behavior, so it should be an error rather than a warning.

An early-return is added, so that we don't even attempt to make the modification. This will make the behavior consistent, and easier to understand.

Also updates the normalization logic to use the resolver's own logic. This will handle all sorts of normalization in addition to our deprecations.
This commit is contained in:
David Taylor 2024-08-09 16:45:33 +01:00 committed by GitHub
parent 206bbb4255
commit b59b24884f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 7 additions and 11 deletions

View File

@ -115,10 +115,6 @@ const DEPRECATED_MODULES = new Map(
}) })
); );
export function normalizeDeprecatedModule(name) {
return DEPRECATED_MODULES.get(name)?.newName || name;
}
export function setResolverOption(name, value) { export function setResolverOption(name, value) {
_options[name] = value; _options[name] = value;
} }

View File

@ -159,7 +159,6 @@ import {
registerIconRenderer, registerIconRenderer,
replaceIcon, replaceIcon,
} from "discourse-common/lib/icon-library"; } from "discourse-common/lib/icon-library";
import { normalizeDeprecatedModule } from "discourse-common/resolver";
import { addImageWrapperButton } from "discourse-markdown-it/features/image-controls"; import { addImageWrapperButton } from "discourse-markdown-it/features/image-controls";
import { CUSTOM_USER_SEARCH_OPTIONS } from "select-kit/components/user-chooser"; import { CUSTOM_USER_SEARCH_OPTIONS } from "select-kit/components/user-chooser";
import { modifySelectKit } from "select-kit/mixins/plugin-api"; import { modifySelectKit } from "select-kit/mixins/plugin-api";
@ -254,26 +253,27 @@ class PluginApi {
_resolveClass(resolverName, opts) { _resolveClass(resolverName, opts) {
opts = opts || {}; opts = opts || {};
const normalized = this.container.registry.normalize(resolverName);
if ( if (
this.container.cache[normalizeDeprecatedModule(resolverName)] || this.container.cache[normalized] ||
(resolverName === "model:user" && (normalized === "model:user" &&
this.container.lookup("service:current-user")) this.container.lookup("service:current-user"))
) { ) {
// eslint-disable-next-line no-console // eslint-disable-next-line no-console
console.warn( console.error(
consolePrefix(), consolePrefix(),
`Attempted to modify "${resolverName}", but it was already initialized earlier in the boot process (e.g. via a lookup()). Remove that lookup, or move the modifyClass call earlier in the boot process for changes to take effect. https://meta.discourse.org/t/262064` `Attempted to modify "${resolverName}", but it was already initialized earlier in the boot process (e.g. via a lookup()). Remove that lookup, or move the modifyClass call earlier in the boot process for changes to take effect. https://meta.discourse.org/t/262064`
); );
return;
} }
const klass = this.container.factoryFor(resolverName); const klass = this.container.factoryFor(normalized);
if (!klass) { if (!klass) {
if (!opts.ignoreMissing) { if (!opts.ignoreMissing) {
// eslint-disable-next-line no-console // eslint-disable-next-line no-console
console.warn( console.warn(
consolePrefix(), consolePrefix(),
`"${resolverName}" was not found by modifyClass` `"${normalized}" was not found by modifyClass`
); );
} }
return; return;