Skip to content

DEV: Make outletArgs available as regular arguments #32819

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 22, 2025
Merged

DEV: Make outletArgs available as regular arguments #32819

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
285320e
DEV: Make outletArgs available as regular arguments
davidtaylorhq May 20, 2025
84302c0
lint
davidtaylorhq May 22, 2025
ef9bd41
deprecated args can be lazy too
davidtaylorhq May 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions app/assets/javascripts/discourse/app/components/header/contents.gjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import Component from "@glimmer/component";
import { hash } from "@ember/helper";
import { service } from "@ember/service";
import { and } from "truth-helpers";
import deprecatedOutletArgument from "discourse/helpers/deprecated-outlet-argument";
Expand Down Expand Up @@ -70,7 +69,7 @@ export default class Contents extends Component {
topicInfo=@topicInfo
topicInfoVisible=@topicInfoVisible
}}
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
topic=(deprecatedOutletArgument
value=this.header.topic
message="The argument 'topic' is deprecated on the outlet 'header-contents__before', use 'topicInfo' or 'topicInfoVisible' instead"
Expand Down Expand Up @@ -122,7 +121,7 @@ export default class Contents extends Component {
topicInfo=@topicInfo
topicInfoVisible=@topicInfoVisible
}}
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
topic=(deprecatedOutletArgument
value=this.header.topic
message="The argument 'topic' is deprecated on the outlet 'before-header-panel', use 'topicInfo' or 'topicInfoVisible' instead"
Expand All @@ -142,7 +141,7 @@ export default class Contents extends Component {
topicInfo=@topicInfo
topicInfoVisible=@topicInfoVisible
}}
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
topic=(deprecatedOutletArgument
value=this.header.topic
message="The argument 'topic' is deprecated on the outlet 'after-header-panel', use 'topicInfo' or 'topicInfoVisible' instead"
Expand All @@ -160,7 +159,7 @@ export default class Contents extends Component {
topicInfo=@topicInfo
topicInfoVisible=@topicInfoVisible
}}
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
topic=(deprecatedOutletArgument
value=this.header.topic
message="The argument 'topic' is deprecated on the outlet 'header-contents__after', use 'topicInfo' or 'topicInfoVisible' instead"
Expand Down
38 changes: 35 additions & 3 deletions app/assets/javascripts/discourse/app/components/plugin-outlet.gjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import Component from "@glimmer/component";
import { cached } from "@glimmer/tracking";
import ClassicComponent from "@ember/component";
import { concat } from "@ember/helper";
import { get } from "@ember/object";
import { getOwner } from "@ember/owner";
import { service } from "@ember/service";
import curryComponent from "ember-curry-component";
import { or } from "truth-helpers";
import PluginConnector from "discourse/components/plugin-connector";
import PluginOutlet from "discourse/components/plugin-outlet";
Expand Down Expand Up @@ -124,6 +126,29 @@ export default class PluginOutletComponent extends Component {
);
}

@bind
safeCurryComponent(component, args) {
if (component.prototype instanceof ClassicComponent) {
for (const arg of Object.keys(args)) {
if (component.prototype.hasOwnProperty(arg)) {
deprecated(
`Unable to set @${arg} on connector for ${this.args.name}, because a property on the component class clashes with the argument name. Resolve the clash, or convert to a glimmer component.`,
{
id: "discourse.plugin-outlet-classic-args-clash",
}
);

// Build a clone of `args`, without the offending key, while preserving getters
const descriptors = Object.getOwnPropertyDescriptors(args);
delete descriptors[arg];
args = Object.defineProperties({}, descriptors);
}
}
}

return curryComponent(component, args, getOwner(this));
}

<template>
{{~#if (this.connectorsExist hasBlock=(has-block))~}}
{{~#if (has-block)~}}
Expand All @@ -135,9 +160,16 @@ export default class PluginOutletComponent extends Component {

{{~#each (this.getConnectors hasBlock=(has-block)) as |c|~}}
{{~#if c.componentClass~}}
<c.componentClass
@outletArgs={{this.outletArgsWithDeprecations}}
>{{yield}}</c.componentClass>
{{~#let
(this.safeCurryComponent
c.componentClass this.outletArgsWithDeprecations
)
as |CurriedComponent|
~}}
<CurriedComponent
@outletArgs={{this.outletArgsWithDeprecations}}
>{{yield}}</CurriedComponent>
{{~/let~}}
{{~else if @defaultGlimmer~}}
<c.templateOnly
@outletArgs={{this.outletArgsWithDeprecations}}
Expand Down
6 changes: 5 additions & 1 deletion app/assets/javascripts/discourse/app/lib/plugin-connectors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,11 @@ export function buildArgsWithDeprecations(args, deprecatedArgs, opts = {}) {

if (args) {
Object.keys(args).forEach((key) => {
Object.defineProperty(output, key, { value: args[key] });
Object.defineProperty(output, key, {
get() {
return args[key];
},
});
});
}

Expand Down
1 change: 1 addition & 0 deletions app/assets/javascripts/discourse/app/services/deprecation-warning-handler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export const CRITICAL_DEPRECATIONS = [
"discourse.mobile-view",
"discourse.mobile-templates",
"discourse.component-template-overrides",
"discourse.plugin-outlet-classic-args-clash",
];

if (DEBUG) {
Expand Down
4 changes: 2 additions & 2 deletions app/assets/javascripts/discourse/app/static/dev-tools/plugin-outlet-debug/args-table.gjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default class ArgsTable extends Component {
window[`arg${globalI}`] = value;
/* eslint-disable no-console */
console.log(
`[plugin outlet debug] \`@outletArgs.${key}\` saved to global \`arg${globalI}\`, and logged below:`
`[plugin outlet debug] \`@${key}\` saved to global \`arg${globalI}\`, and logged below:`
);
console.log(value);
/* eslint-enable no-console */
Expand All @@ -53,7 +53,7 @@ export default class ArgsTable extends Component {

<template>
{{#each this.renderArgs as |arg|}}
<div class="key"><span class="fw">{{arg.key}}</span>:</div>
<div class="key"><span class="fw">@{{arg.key}}</span>:</div>
<div class="value">
<span class="fw">{{arg.value}}</span>
<a
Expand Down
143 changes: 132 additions & 11 deletions app/assets/javascripts/discourse/tests/integration/components/plugin-outlet-test.gjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Component from "@glimmer/component";
import { tracked } from "@glimmer/tracking";
import ClassicComponent from "@ember/component";
import templateOnly from "@ember/component/template-only";
import { hash } from "@ember/helper";
import { getOwner } from "@ember/owner";
import { click, find, render, settled } from "@ember/test-helpers";
import hbs from "htmlbars-inline-precompile";
Expand Down Expand Up @@ -464,7 +465,7 @@ module("Integration | Component | plugin-outlet", function (hooks) {
<template>
<PluginOutlet
@name="test-name"
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
shouldDisplay=(deprecatedOutletArgument value=true)
}}
/>
Expand Down Expand Up @@ -492,7 +493,7 @@ module("Integration | Component | plugin-outlet", function (hooks) {
<template>
<PluginOutlet
@name="test-name"
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
shouldDisplay=(deprecatedOutletArgument
value=true
message="The 'shouldDisplay' is deprecated on this test"
Expand Down Expand Up @@ -555,7 +556,7 @@ module("Integration | Component | plugin-outlet", function (hooks) {
<template>
<PluginOutlet
@name="test-name"
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
shouldDisplay=(deprecatedOutletArgument
value=deprecatedData.display
silence="discourse.deprecation-that-should-not-be-logged"
Expand Down Expand Up @@ -597,7 +598,7 @@ module("Integration | Component | plugin-outlet", function (hooks) {
<PluginOutlet
@name="test-name"
@outletArgs={{lazyHash shouldDisplay=true}}
@deprecatedArgs={{hash
@deprecatedArgs={{lazyHash
argNotUsed=(deprecatedOutletArgument value=true)
}}
/>
Expand Down Expand Up @@ -788,7 +789,7 @@ module(

test("using classic PluginConnector by default", async function (assert) {
await render(hbs`
<PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
<PluginOutlet @name="test-name" @deprecatedArgs={{lazyHash hello=(deprecated-outlet-argument value="world")}} />
`);

// deprecated argument still works
Expand Down Expand Up @@ -818,7 +819,7 @@ module(
await render(hbs`
<PluginOutlet
@name="test-name"
@deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}}
@deprecatedArgs={{lazyHash hello=(deprecated-outlet-argument value="world")}}
@defaultGlimmer={{true}}
/>
`);
Expand Down Expand Up @@ -858,7 +859,7 @@ module(
});

await render(hbs`
<PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
<PluginOutlet @name="test-name" @deprecatedArgs={{lazyHash hello=(deprecated-outlet-argument value="world")}} />
`);

// deprecated argument still works
Expand Down Expand Up @@ -895,7 +896,7 @@ module(
);

await render(hbs`
<PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
<PluginOutlet @name="test-name" @deprecatedArgs={{lazyHash hello=(deprecated-outlet-argument value="world")}} />
`);

// deprecated argument still works
Expand Down Expand Up @@ -928,7 +929,7 @@ module(
);

await render(hbs`
<PluginOutlet @name="test-name" @deprecatedArgs={{hash hello=(deprecated-outlet-argument value="world")}} />
<PluginOutlet @name="test-name" @deprecatedArgs={{lazyHash hello=(deprecated-outlet-argument value="world")}} />
`);

// deprecated argument still works
Expand All @@ -950,7 +951,7 @@ module(

test("unused arguments", async function (assert) {
await render(hbs`
<PluginOutlet @name="test-name" @outletArgs={{lazyHash hello="world"}} @deprecatedArgs={{hash argNotUsed=(deprecated-outlet-argument value="not used")}} />
<PluginOutlet @name="test-name" @outletArgs={{lazyHash hello="world"}} @deprecatedArgs={{lazyHash argNotUsed=(deprecated-outlet-argument value="not used")}} />
`);

// deprecated argument still works
Expand Down Expand Up @@ -1058,6 +1059,126 @@ module(
}
);

module(
"Integration | Component | plugin-outlet | argument currying",
function (hooks) {
setupRenderingTest(hooks);

test("makes arguments available at top level", async function (assert) {
extraConnectorComponent(
"test-name",
<template>
<span class="glimmer-test">{{@arg1}} from glimmer</span>
</template>
);

await render(
<template>
<PluginOutlet
@name="test-name"
@outletArgs={{lazyHash arg1="Hello world"}}
/>
</template>
);
assert.dom(".glimmer-test").hasText("Hello world from glimmer");
});

test("doesn't completely rerender when arguments change", async function (assert) {
const events = [];

extraConnectorComponent(
"test-name",
class extends Component {
constructor() {
super(...arguments);
events.push("constructor");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't really matter here but this check could also be done with assert.step()/assert.verifySteps

}

willDestroy() {
super.willDestroy(...arguments);
events.push("willDestroy");
}

<template>
<span class="glimmer-test">{{@arg1}} from glimmer</span>
</template>
}
);

const testState = new (class {
@tracked hello = "Hello world";
})();

await render(
<template>
<PluginOutlet
@name="test-name"
@outletArgs={{lazyHash arg1=testState.hello}}
/>
</template>
);
assert.dom(".glimmer-test").hasText("Hello world from glimmer");
assert.deepEqual(events, ["constructor"], "constructor called once");

testState.hello = "changed";
await settled();
assert.dom(".glimmer-test").hasText("changed from glimmer");
assert.deepEqual(events, ["constructor"], "constructor not called again");
});

test("makes arguments available at top level in classic components", async function (assert) {
extraConnectorComponent(
"test-name",
class extends ClassicComponent {
<template>
<span class="classic-test">{{this.arg1}} from classic</span>
</template>
}
);

await render(
<template>
<PluginOutlet
@name="test-name"
@outletArgs={{lazyHash arg1="Hello world"}}
/>
</template>
);
assert.dom(".classic-test").hasText("Hello world from classic");
});

test("guards against name clashes in classic components", async function (assert) {
extraConnectorComponent(
"test-name",
class extends ClassicComponent {
get arg1() {
return "overridden";
}

<template>
<span class="classic-test">{{this.arg1}} from classic</span>
</template>
}
);

await withSilencedDeprecationsAsync(
"discourse.plugin-outlet-classic-args-clash",
async () => {
await render(
<template>
<PluginOutlet
@name="test-name"
@outletArgs={{lazyHash arg1="Hello world"}}
/>
</template>
);
}
);
assert.dom(".classic-test").hasText("overridden from classic");
});
}
);

module(
"Integration | Component | plugin-outlet | whitespace",
function (hooks) {
Expand Down
Loading