Skip to content

Add guideline for defining the name of a controller#8528

Merged
mcmire merged 7 commits intomainfrom
add-guideline-for-controller-name
Apr 23, 2026
Merged

Add guideline for defining the name of a controller#8528
mcmire merged 7 commits intomainfrom
add-guideline-for-controller-name

Conversation

@mcmire
Copy link
Copy Markdown
Contributor

@mcmire mcmire commented Apr 20, 2026

Explanation

There is no written standard for defining the name of a controller, and some contributors do not have a common understanding of the do's and dont's for the name.

This commit adds such a guideline, highlighting the following points:

  • The name of a controller should be defined in a variable called CONTROLLER_NAME (formerly controllerName)
  • The name should be used to define custom actions and events and should be passed to BaseController
  • The name should not be exported from the package

Besides updating the guidelines, this commit also updates the sample-controllers package to follow them, most notably changing controllerName to CONTROLLER_NAME and removing the export from this constant.

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Removes an exported constant and renames controllerName to CONTROLLER_NAME in sample controllers, which could break any consumers importing that symbol even though runtime behavior is unchanged.

Overview
Adds a new controller guideline requiring a single internal CONTROLLER_NAME constant to namespace messenger actions/events and BaseController state, and explicitly discouraging exporting that constant from the package.

Updates the sample-controllers examples (sample-gas-prices-controller and sample-petnames-controller) to follow the guideline by replacing exported controllerName with an internal CONTROLLER_NAME and wiring all messenger/type and BaseController references to it.

Reviewed by Cursor Bugbot for commit 4757fdb. Bugbot is set up for automated code reviews on this repo. Configure here.

There is no written standard for defining the name of a controller, and
some contributors do not have a common understanding of the do's and
dont's for the name.

This commit adds such a guideline, highlighting the following points:

- The name of a controller should be defined in a variable called
  `CONTROLLER_NAME` (formerly `controllerName`)
- The name should be used to define custom actions and events and should
  be passed to `BaseController`
- The name should not be exported from the package
@mcmire
Copy link
Copy Markdown
Contributor Author

mcmire commented Apr 20, 2026

Adding no-changelog because this is a non-public-facing change to sample-controllers.

@mcmire mcmire marked this pull request as ready for review April 20, 2026 17:34
@mcmire mcmire requested a review from a team as a code owner April 20, 2026 17:34

Every controller has a name, which is not only used to namespace the controller's messenger actions and events, but also to namespace the controller's state data when composed with other controllers.

The name should be defined in a constant called `CONTROLLER_NAME` so that it can be easily changed if the need arises. (This variable was formerly called `controllerName`.) The name should be used to initialize the messenger, and should also be passed to the `BaseController` constructor.
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.

Probably not necessary to include this.

Suggested change
The name should be defined in a constant called `CONTROLLER_NAME` so that it can be easily changed if the need arises. (This variable was formerly called `controllerName`.) The name should be used to initialize the messenger, and should also be passed to the `BaseController` constructor.
The name should be defined in a constant called `CONTROLLER_NAME` so that it can be easily changed if the need arises. The name should be used to initialize the messenger, and should also be passed to the `BaseController` constructor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in b450e6d.


const CONTROLLER_NAME = 'FooController';

export type FooControllerSomeCustomAction = {
Copy link
Copy Markdown
Member

@Mrtenz Mrtenz Apr 21, 2026

Choose a reason for hiding this comment

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

This example doesn't really make sense given the MESSENGER_EXPOSED_METHODS pattern. Maybe update it to use ControllerGetStateAction or some events?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right. In fact most of the examples in this document don't make much sense anymore 😅 I will update this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I've updated these examples here: b450e6d. I replaced FooController with a more realistic name and I used events instead of actions for the examples.

@mcmire mcmire enabled auto-merge April 23, 2026 14:27
@mcmire mcmire added this pull request to the merge queue Apr 23, 2026
Merged via the queue into main with commit c906ab9 Apr 23, 2026
358 checks passed
@mcmire mcmire deleted the add-guideline-for-controller-name branch April 23, 2026 14:45
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