Skip to content

[2.x] feat(core): introduce conditional frontend extenders - #5443

Merged
imorland merged 5 commits into
flarum:2.xfrom
glowingblue:di/frontend-conditional-extender
Nov 11, 2026
Merged

imorland merged 5 commits into
flarum:2.xfrom
glowingblue:di/frontend-conditional-extender

Conversation

@DavideIadeluca

@DavideIadeluca DavideIadeluca commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Docs PR flarum/docs#706

Fixes #0000
When an extension only conditionally (e.g. only when another extension is enabled) wanted to use the frontend extenders like Store() or Model() extensions have two awkwardly implement it. In Flarum 1 I've come across three patterns here on how it was solved before this PR

Approach 1: Ternary in extend.ts:

import app from 'flarum/forum/app';
import Extend from 'flarum/common/extenders';
import { models } from '@acme-bar';
import Foo from './models/Foo';

const barExtenders = app.initializers.has('acme-bar')
  ? [
      new Extend.Store() //
        .add('foos', Foo),

      new Extend.Model(models.Bar) //
        .hasMany<Foo>('foos'),
    ]
  : [];

export default [
  // ...

  ...barExtenders,
];

Approach 2: Marking it as undefined in TS:

import Extend from 'flarum/common/extenders';
import Discussion from 'flarum/common/models/Discussion';

export default [
  // ...

  // Only sent by the backend when acme-bar is enabled.
  new Extend.Model(Discussion) //
    .attribute<boolean | undefined>('fooBarFeatured'),
];

Approach 3: Avoid the conditional by creating a pivot extension
Approach three is to create a new dedicated extension which has hard dependencies on the integrating extensions.

Changes proposed in this pull request:
Implement new Conditional frontend extender to work similarly like the backend equivalent

import Extend from 'flarum/common/extenders';
import Bar from 'ext:acme/bar/common/models/Bar';
import Foo from './models/Foo';

export default [
  // ...

  new Extend.Conditional() //
    .whenExtensionEnabled('acme-bar', () => [
      new Extend.Store() //
        .add('foos', Foo),

      new Extend.Model(Bar) //
        .hasMany<Foo>('foos'),
    ]),
];

Reviewers should focus on:
Backwards compatibility / general regression

Screenshot
(No-Op)

Before
before

After
after

Necessity

  • Has the problem that is being solved here been clearly explained?
  • If applicable, have various options for solving this problem been considered?
  • For core PRs, does this need to be in core, or could it be in an extension?
  • Are we willing to maintain this for years / potentially forever?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Frontend changes: tests are green (run yarn test in js/).
  • Frontend changes: tests have been added, or are not appropriate here.
  • Backend changes: tests are green (run composer test).
  • Backend changes: tests have been added, or are not appropriate here.
  • Where applicable, changes are suitable for all supported database drivers (MySQL, MariaDB, PostgreSQL, SQLite).
  • Core developer confirmed locally this works as intended.
  • The description above is written by me and describes what this pull request actually does.

Required changes:

  • Related documentation PR: (Remove if irrelevant)

@DavideIadeluca DavideIadeluca changed the title [1.x] feat: introduce conditional frontend extenders [2.x] feat: introduce conditional frontend extenders Nov 9, 2026
@DavideIadeluca DavideIadeluca changed the title [2.x] feat: introduce conditional frontend extenders [2.x] feat(core): introduce conditional frontend extenders Nov 9, 2026
@DavideIadeluca
DavideIadeluca marked this pull request as ready for review November 9, 2026 13:17
@DavideIadeluca
DavideIadeluca requested a review from a team as a code owner November 9, 2026 13:17
@DavideIadeluca

Copy link
Copy Markdown
Contributor Author

Docs PR flarum/docs#706 ;)

@imorland imorland added this to the 2.1 milestone Nov 10, 2026
@imorland

Copy link
Copy Markdown
Member

The new extender already uses extensionId in flarum.extensions, which is the right check here. One clarification for the “Approach 1” example in the description: app.initializers.has('acme-bar') only checks for an initializer with that name. In 2.0 an extension can export its extenders from extend.ts without registering an initializer at all, and initializer names need not match extension IDs. Please prefer 'acme-bar' in flarum.extensions when illustrating extension detection, with the existing caveat that this only detects JavaScript loaded for the current frontend.

There is one example to fix before merging: the when() JSDoc uses (app) => !!app.forum.attribute('acmeAdvancedMode'). app.bootExtensions() runs before app.boot(), which is where app.forum and app.session are assigned. I reproduced the resulting TypeError with this PR's Conditional class and the pre-boot application state; in the page's boot sequence that exception prevents frontend startup.

Please replace that example with a condition that is available during extension boot and document the timing beside when(). The linked docs PR already explains this restriction correctly. A test exercising the condition callback with the supplied application before app.boot() would help keep the example and lifecycle aligned.

@imorland imorland left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please correct the when() JSDoc example before merging. Its predicate reads app.forum.attribute('acmeAdvancedMode'), but bootExtensions() evaluates it before app.boot() assigns app.forum and app.session. I reproduced the resulting TypeError; copying the advertised example prevents frontend startup.

Replace it with a condition available during extension boot and document that timing beside when(), consistent with the warning already in the linked docs PR. Please also exercise the callback with the supplied application before app.boot() in a test.

As noted in my earlier comment, please use 'acme-bar' in flarum.extensions in the description's detection example: an extension using only extend.ts need not register an initializer. The new extender already uses the correct registry check.

@DavideIadeluca

Copy link
Copy Markdown
Contributor Author

Thanks for the review and pointing out the (in retrospect) obvious flaw in the when() method and it's JSDoc. I have now done the following changes:

  1. Updated the JSDoc example for the when() method to a working example
  2. Introduced a new whenForumAttribute() method which can be used instead to check a forum attributes truthiness
  3. Adopted the plural extension enabled/disabled checks to have parity with the backend now that [2.x] feat(core): add plural extension checks to the Conditional extender #5444) got merged
  4. Updated the Docs PR to reflect the latest changes docs: document the frontend Conditional extender docs#706
  5. Updated the PR description to clarify that the approaches I have come across on refer to Flarum 1 extensions

@imorland May you please now have another look?

@imorland
imorland merged commit 36ff8d1 into flarum:2.x Nov 11, 2026
12 checks passed
@imorland
imorland deleted the di/frontend-conditional-extender branch November 11, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants