-
Notifications
You must be signed in to change notification settings - Fork 49
fpm-writer - Update ControllerExtension.d.ts for easier usage #3658
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?
Conversation
🦋 Changeset detectedLatest commit: c704543 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
… into update_ControllerExtension.d.ts * origin/update_ControllerExtension.d.ts: Create serious-beans-dream.md
|
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.
See comments. Coming with proper types out-of-the box would really be a great benefit for TS based controller extensions. So thanks for this PR 👍🏻
|
||
export default class ControllerExtension<ExtensionAPI> { | ||
override?: Overrides; | ||
base: { |
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.
Actually I use
import type PageController from "sap/fe/core/PageController";
//[...]
protected base!: PageController;
as type in my controller extension. I don't think you need to list all PageController functions if you just re-use that type. And it also comes with less maintenance in case the PageController type changes later on.
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.
Another thing that came into my mind. Actually base can be the actual controller that is extended (ListReportController or ObjectPageController), that itself returns the respective ExtensionAPI (LR or OP ExtensionAPI), and some additional APIs.
https://ui5.sap.com/#/api/sap.fe.templates
The ObjectPageController is missing from our public API, but I'd say we should make it public as well (as it can also be extended already) - any objections @nlunets ?
}; | ||
} | ||
|
||
export type Overrides<GenericController extends Controller = Controller> = { |
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.
For the overrides I use
import ViewState from "sap/fe/core/controllerextensions/ViewState";
import EditFlow from "sap/fe/core/controllerextensions/EditFlow";
import Routing from "sap/fe/core/controllerextensions/Routing";
import Controller from "sap/ui/core/mvc/Controller";
type ControllerExtensionOverrides = Partial<Pick<typeof Controller["prototype"], "onAfterRendering" | "onBeforeRendering" | "onExit" | "onInit">>;
/**
* Types for Fiori Elements Controller Extension Overrides (sap.ui.core.mvc.ControllerExtension.override).
*/
type FioriElementsControllerExtensionOverrides = ControllerExtensionOverrides & {
/**
* experimental
*/
onPageReady?: () => void;
editFlow?: Partial<Pick<typeof EditFlow["prototype"], "onAfterCreate" | "onAfterDelete" | "onAfterDiscard" | "onAfterEdit" | "onAfterSave" | "onBeforeCreate" | "onBeforeDelete" | "onBeforeDiscard" | "onBeforeEdit" | "onBeforeSave">>;
routing?: Partial<Pick<typeof Routing["prototype"], "onAfterBinding" | "onBeforeBinding" | "onBeforeNavigation">>;
viewState?: Partial<Pick<typeof ViewState["prototype"], "adaptBindingRefreshControls" | "adaptBindingRefreshHandler" | "adaptControlStateHandler" |"adaptStateControls" | "applyAdditionalStates" | "applyInitialStateOnly" | "applyNavigationParameters" | "onAfterStateApplied" | "onBeforeStateApplied" | "retrieveAdditionalStates">>;
};
in my controller extension.
My definition might be incomplete though. But I think just having every property optional actually is not the correct type for overrides because you cannot override everything from that type. So code completion gives you wrong suggestions. Downside is of course that newly introduced properties are missing in the type definition.
To overcome this nowadays I probably would introduce a utility type to filter on e.g. event handers that start with on
and pick those.
/**
* A utility type that extracts the keys of an object `T` that correspond to methods
* starting with the prefix "on".
*/
type OnMethodKeys<T> = {
[K in keyof T]: K extends `on${string}` // 1. If the key starts with "on"...
? T[K] extends (...args: any[]) => any // 2. ...and the property is a function...
? K // ...keep the key.
: never
: never; // 3. Discard all other keys.
}[keyof T]; // 4. Collect the kept keys into a union type.
//[...]
editFlow?: Partial<Pick<typeof EditFlow["prototype"], OnMethodKeys<typeof EditFlow["prototype"]>>>;
@MarcelWaechter But actually I would prefer if Fiori elements would provide those types (UI5 version dependent). This would be a great feature, though I understand the JSDoc-based type generation is a hurdle 🙈
(Please note that onPageReady
is experimental and should maybe not be part in an official type definition.)
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.
yeah onPageReady is not a public one. Recommendations also not yet public.
EditFlow, IntentBasedNavigation, MessageHandler, Paginator, Routing, Share and ViewState is fine.
Yeah let's see how we can improve as well, also adding @nlunets .
Now as we want to provide the Fiori Dev Portal examples with TypeScript, we have the same pain 😆
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 further refined this. What could be handy is something like:
import ViewState from "sap/fe/core/controllerextensions/ViewState";
import EditFlow from "sap/fe/core/controllerextensions/EditFlow";
import Routing from "sap/fe/core/controllerextensions/Routing";
import Controller from "sap/ui/core/mvc/Controller";
/**
* A utility type that extracts the keys of an object `T` that correspond to methods
* starting with the specified prefix `P`.
*
* @template T - The object type to extract method keys from
* @template P - The string prefix to match (e.g., "on", "adapt", "apply", "retrieve")
*/
type MethodKeys<T, P extends string> = {
[K in keyof T]: K extends `${P}${string}` // 1. If the key starts with the prefix...
? T[K] extends (...args: any[]) => any // 2. ...and the property is a function...
? K // ...keep the key.
: never
: never; // 3. Discard all other keys.
}[keyof T]; // 4. Collect the kept keys into a union type.
/**
* A utility type that combines method keys with multiple prefixes.
*
* @template T - The object type to extract method keys from
* @template P - A union of string prefixes to match
*/
type MultiPrefixMethodKeys<T, P extends string> = P extends string
? MethodKeys<T, P>
: never;
/**
* Types for UI5 Controller Extension Overrides (sap.ui.core.mvc.ControllerExtension.override).
*/
type ControllerExtensionOverrides = Partial<Pick<typeof Controller["prototype"], MethodKeys<typeof Controller["prototype"], "on">>>;
/**
* Types for Fiori Elements Controller Extension Overrides (sap.ui.core.mvc.ControllerExtension.override).
*/
type FioriElementsControllerExtensionOverrides = ControllerExtensionOverrides & {
/**
* experimental
*/
onPageReady?: () => void;
editFlow?: Partial<Pick<typeof EditFlow["prototype"], MethodKeys<typeof EditFlow["prototype"], "on">>>;
routing?: Partial<Pick<typeof Routing["prototype"], MethodKeys<typeof Routing["prototype"], "on">>>;
viewState?: Partial<Pick<typeof ViewState["prototype"], MultiPrefixMethodKeys<typeof ViewState["prototype"], "on" | "adapt" | "apply" | "retrieve">>>;
};
If Fiori elements keeps following those function naming pattern this should be fine.
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.
not sure, the fact a method starts with on doesn't say it's public and can be overridden. That's defined in the decorators, like
@publicExtension()
@extensible("AfterAsync")
async onBeforeSave(_mParameters?: { context?: Context }): Promise {
// to be overridden
return Promise.resolve();
}
I guess you can't read our decorators... but it's there in the metadata, just checked quickly

but yes not great, it shall be ideally accessible through the TS types...
No description provided.