-
-
Notifications
You must be signed in to change notification settings - Fork 825
Move ReactiveController and Host to core #6548
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -115,4 +115,5 @@ const KEEP_IMPORTS = new Set([ | |
| 'jsx', | ||
| 'jsxs', | ||
| 'jsxDEV', | ||
| 'ReactiveControllerHost', | ||
| ]); | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -638,6 +638,47 @@ export interface ComponentInterface { | |||||||||||||||||||||
| [memberName: string]: any; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Interface for reactive controllers that can be attached to a ReactiveControllerHost. | ||||||||||||||||||||||
| * Controllers implement lifecycle hooks that are called by the host during component lifecycle. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| export interface ReactiveController { | ||||||||||||||||||||||
| hostConnected?(): void; | ||||||||||||||||||||||
| hostDisconnected?(): void; | ||||||||||||||||||||||
| hostWillLoad?(): Promise<void> | void; | ||||||||||||||||||||||
| hostDidLoad?(): void; | ||||||||||||||||||||||
| hostWillRender?(): Promise<void> | void; | ||||||||||||||||||||||
| hostDidRender?(): void; | ||||||||||||||||||||||
| hostWillUpdate?(): Promise<void> | void; | ||||||||||||||||||||||
| hostDidUpdate?(): void; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * Base class that implements ComponentInterface and provides reactive controller functionality. | ||||||||||||||||||||||
| * Components can extend this class to enable reactive controller composition. | ||||||||||||||||||||||
| * | ||||||||||||||||||||||
| * Known Limitation: Components extending ReactiveControllerHost cannot use | ||||||||||||||||||||||
| * `<Host>` as their root element in the render method. This is because | ||||||||||||||||||||||
| * ReactiveControllerHost does not extend HTMLElement. Instead, return a | ||||||||||||||||||||||
| * regular element (like `<div>`) as the root. | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| export declare class ReactiveControllerHost implements ComponentInterface { | ||||||||||||||||||||||
| controllers: Set<ReactiveController>; | ||||||||||||||||||||||
|
||||||||||||||||||||||
| controllers: Set<ReactiveController>; | |
| readonly controllers: Set<ReactiveController>; |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type declaration for lifecycle methods doesn't match the implementation. The componentWillLoad, componentWillRender, and componentWillUpdate methods should be declared with return type Promise<void> | void to match the ComponentInterface and properly handle async operations. Currently they're declared as returning void only.
| componentWillLoad(): void; | |
| componentDidLoad(): void; | |
| componentWillRender(): void; | |
| componentDidRender(): void; | |
| componentWillUpdate(): void; | |
| componentWillLoad(): Promise<void> | void; | |
| componentDidLoad(): void; | |
| componentWillRender(): Promise<void> | void; | |
| componentDidRender(): void; | |
| componentWillUpdate(): Promise<void> | void; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ComponentInterface } from '../declarations/stencil-public-runtime'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { forceUpdate } from './update-component'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface ReactiveController { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostConnected?(): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostDisconnected?(): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostWillLoad?(): Promise<void> | void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostDidLoad?(): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostWillRender?(): Promise<void> | void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostDidRender?(): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| hostWillUpdate?(): Promise<void> | void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+11
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface ReactiveController { | |
| hostConnected?(): void; | |
| hostDisconnected?(): void; | |
| hostWillLoad?(): Promise<void> | void; | |
| hostDidLoad?(): void; | |
| hostWillRender?(): Promise<void> | void; | |
| hostDidRender?(): void; | |
| hostWillUpdate?(): Promise<void> | void; | |
| export interface ReactiveController { | |
| /** | |
| * Called when the host component's `connectedCallback` is invoked. | |
| * Use this to perform setup that depends on the host being connected to the DOM. | |
| */ | |
| hostConnected?(): void; | |
| /** | |
| * Called when the host component's `disconnectedCallback` is invoked. | |
| * Use this to perform cleanup when the host is removed from the DOM. | |
| */ | |
| hostDisconnected?(): void; | |
| /** | |
| * Called when the host component's `componentWillLoad` lifecycle runs. | |
| * Use this to perform work before the host first renders. | |
| */ | |
| hostWillLoad?(): Promise<void> | void; | |
| /** | |
| * Called when the host component's `componentDidLoad` lifecycle runs. | |
| * Use this to perform work after the host has finished its initial render. | |
| */ | |
| hostDidLoad?(): void; | |
| /** | |
| * Called when the host component's `componentWillRender` lifecycle runs. | |
| * Use this to react to state changes before each render. | |
| */ | |
| hostWillRender?(): Promise<void> | void; | |
| /** | |
| * Called when the host component's `componentDidRender` lifecycle runs. | |
| * Use this to perform work after each render. | |
| */ | |
| hostDidRender?(): void; | |
| /** | |
| * Called when the host component's `componentWillUpdate` lifecycle runs. | |
| * Use this to react to prop or state changes before an update. | |
| */ | |
| hostWillUpdate?(): Promise<void> | void; | |
| /** | |
| * Called when the host component's `componentDidUpdate` lifecycle runs. | |
| * Use this to perform work after an update has been applied. | |
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this limitation (/ is it acceptable?) - can you explain it to me?
<Host> is just a jsx helper, allowing devs to target and sprout attributes on the output <custom-element> tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a component extends ReactiveControllerHost and uses as the root element, Stencil's runtime expects the component instance to be the actual custom element (HTMLElement), but ReactiveControllerHost doesn't satisfy that contract, leading to initialization failures. When I tried to make the ReactiveControllerHost extend HTMLElement I hit some initialization issues
Runtime error : custom-elements#externalruntime: undefined
When ReactiveControllerHost extends HTMLElement and is imported from @stencil/core, the Stencil compiler's lazy loading mechanism fails to properly initialize component instances. The runtime thinks these are external custom-element components that need the externalRuntime flag, but they're actually regular Stencil components.
If you have tips on how to address this it would be awesome.
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The controllers property is declared as public without any access modifier or documentation. Consider making it readonly to prevent external modification of the Set, as the API provides addController and removeController methods for managing controllers. This would ensure the Set is only modified through the intended API surface.
| controllers = new Set<ReactiveController>(); | |
| readonly controllers = new Set<ReactiveController>(); |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lifecycle methods componentWillLoad, componentWillRender, and componentWillUpdate do not properly handle Promise return values from controller hooks. The controller interface allows these methods to return Promise<void> | void, but the host implementation doesn't await these promises. This could lead to race conditions where lifecycle execution continues before async controller operations complete. Consider using Promise.all() to await all controller promises before proceeding.
| componentWillLoad() { | |
| this.controllers.forEach((controller) => controller.hostWillLoad?.()); | |
| } | |
| componentDidLoad() { | |
| this.controllers.forEach((controller) => controller.hostDidLoad?.()); | |
| } | |
| componentWillRender() { | |
| this.controllers.forEach((controller) => controller.hostWillRender?.()); | |
| } | |
| componentDidRender() { | |
| this.controllers.forEach((controller) => controller.hostDidRender?.()); | |
| } | |
| componentWillUpdate() { | |
| this.controllers.forEach((controller) => controller.hostWillUpdate?.()); | |
| componentWillLoad(): Promise<void> | void { | |
| const promises: Promise<void>[] = []; | |
| this.controllers.forEach((controller) => { | |
| const result = controller.hostWillLoad?.(); | |
| if (result instanceof Promise) { | |
| promises.push(result); | |
| } | |
| }); | |
| if (promises.length > 0) { | |
| return Promise.all(promises).then(() => undefined); | |
| } | |
| } | |
| componentDidLoad() { | |
| this.controllers.forEach((controller) => controller.hostDidLoad?.()); | |
| } | |
| componentWillRender(): Promise<void> | void { | |
| const promises: Promise<void>[] = []; | |
| this.controllers.forEach((controller) => { | |
| const result = controller.hostWillRender?.(); | |
| if (result instanceof Promise) { | |
| promises.push(result); | |
| } | |
| }); | |
| if (promises.length > 0) { | |
| return Promise.all(promises).then(() => undefined); | |
| } | |
| } | |
| componentDidRender() { | |
| this.controllers.forEach((controller) => controller.hostDidRender?.()); | |
| } | |
| componentWillUpdate(): Promise<void> | void { | |
| const promises: Promise<void>[] = []; | |
| this.controllers.forEach((controller) => { | |
| const result = controller.hostWillUpdate?.(); | |
| if (result instanceof Promise) { | |
| promises.push(result); | |
| } | |
| }); | |
| if (promises.length > 0) { | |
| return Promise.all(promises).then(() => undefined); | |
| } |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lifecycle methods in ReactiveControllerHost don't include error handling when invoking controller hooks. If a controller's lifecycle hook throws an error, it could prevent other controllers from executing their hooks. Consider wrapping controller hook calls in try-catch blocks to ensure one controller's error doesn't affect others.
| connectedCallback() { | |
| this.controllers.forEach((controller) => controller.hostConnected?.()); | |
| } | |
| disconnectedCallback() { | |
| this.controllers.forEach((controller) => controller.hostDisconnected?.()); | |
| } | |
| componentWillLoad() { | |
| this.controllers.forEach((controller) => controller.hostWillLoad?.()); | |
| } | |
| componentDidLoad() { | |
| this.controllers.forEach((controller) => controller.hostDidLoad?.()); | |
| } | |
| componentWillRender() { | |
| this.controllers.forEach((controller) => controller.hostWillRender?.()); | |
| } | |
| componentDidRender() { | |
| this.controllers.forEach((controller) => controller.hostDidRender?.()); | |
| } | |
| componentWillUpdate() { | |
| this.controllers.forEach((controller) => controller.hostWillUpdate?.()); | |
| } | |
| componentDidUpdate() { | |
| this.controllers.forEach((controller) => controller.hostDidUpdate?.()); | |
| private invokeControllerHook(hook: keyof ReactiveController) { | |
| this.controllers.forEach((controller) => { | |
| const callback = controller[hook]; | |
| if (typeof callback === 'function') { | |
| try { | |
| callback.call(controller); | |
| } catch (error) { | |
| // Ensure one controller's error doesn't prevent others from running | |
| console.error( | |
| `Error in ReactiveController ${String(hook)} hook:`, | |
| error, | |
| ); | |
| } | |
| } | |
| }); | |
| } | |
| connectedCallback() { | |
| this.invokeControllerHook('hostConnected'); | |
| } | |
| disconnectedCallback() { | |
| this.invokeControllerHook('hostDisconnected'); | |
| } | |
| componentWillLoad() { | |
| this.invokeControllerHook('hostWillLoad'); | |
| } | |
| componentDidLoad() { | |
| this.invokeControllerHook('hostDidLoad'); | |
| } | |
| componentWillRender() { | |
| this.invokeControllerHook('hostWillRender'); | |
| } | |
| componentDidRender() { | |
| this.invokeControllerHook('hostDidRender'); | |
| } | |
| componentWillUpdate() { | |
| this.invokeControllerHook('hostWillUpdate'); | |
| } | |
| componentDidUpdate() { | |
| this.invokeControllerHook('hostDidUpdate'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for the ReactiveController interface is minimal. Consider adding JSDoc comments explaining the purpose and usage of each lifecycle hook method, similar to how ComponentInterface methods are documented elsewhere in the codebase. This would help developers understand when each hook is called and what they should be used for.