diff --git a/README.md b/README.md index 33f3292..12d5fe7 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ The UI5 [Model Context Protocol](https://modelcontextprotocol.io/) server offers - `get_project_info`: Extracts metadata and configuration from a UI5 project. - `get_version_info`: Retrieves version information for the UI5 framework. - `run_ui5_linter`: Integrates with [`@ui5/linter`](https://github.com/UI5/linter) to analyze and report issues in UI5 code. +- `get_typescript_conversion_guidelines`: Provides guidelines for converting UI5 applications and controls from JavaScript to TypeScript. - `get_integration_cards_guidelines`: Provides access to UI Integration Cards development best practices. - `create_integration_card`: Scaffolds a new UI Integration Card. diff --git a/resources/typescript_conversion_guidelines.md b/resources/typescript_conversion_guidelines.md new file mode 100644 index 0000000..9f60b5b --- /dev/null +++ b/resources/typescript_conversion_guidelines.md @@ -0,0 +1,990 @@ +# UI5 TypeScript Conversion Guidelines + +> *This document outlines how a UI5 (SAPUI5/OpenUI5) project can be converted to TypeScript. The first part explains how the setup of the project needs to be changed, the second part deals with converting the code itself.* + + +## General Conversion Rules + +### Preserve ALL comments + +You MUST preserve existing JSDoc, documentation and comments - never remove JSDoc or comments during the conversion. + +Example input: + +```js +/** + * My cool controller, it does things. + */ +return Controller.extend("com.myorg.myapp.controller.BaseController", { + /** + * Convenience method for accessing the component of the controller's view. + * @returns {sap.ui.core.Component} The component of the controller's view + */ + getOwnerComponent: function () { + // comment + return Controller.prototype.getOwnerComponent.call(this); + }, + ... +}); +``` + +Wrong output: + +```ts +export default class BaseController extends Controller { + public getOwnerComponent(): UIComponent { + return super.getOwnerComponent() as UIComponent; + } +} +``` + +Correct output: + +```ts +/** + * My cool controller, it does things. + * @namespace com.myorg.myapp.controller + */ +export default class BaseController extends Controller { + /** + * Convenience method for accessing the component of the controller's view. + * @returns {sap.ui.core.Component} The component of the controller's view + */ + public getOwnerComponent(): UIComponent { + // comment + return super.getOwnerComponent() as UIComponent; + } +} +``` + +### Be diligent + +Carefully respect all guidelines in this document (and adapt appropriately where required). Before each conversion step, consider all relevant details from this document. + +### Go step-by-step + +You should convert the project step by step, starting with the TypeScript project setup and then the most central files on which other files depend, so those other files can use the typed version of those central files once they are converted as well. `"allowJs": true` in the `tsconfig.json`'s `compilerOptions` may be useful to run semi-converted projects if needed. + +### Avoid `any` type + +Do not take shortcuts, but try to find the proper type or create an interface instead of `any`. + +BAD: +```ts +(this.getOwnerComponent() as any).getContentDensityClass(); +``` + +GOOD: +```ts +(this.getOwnerComponent() as AppComponent).getContentDensityClass() +``` + +### Avoid `unknown` casts + +Import and use actual UI5 control types instead (either the base class `sap/ui/core/Control` or more specific classes if needed to access the respective property). Inspect the XMLView to find out which control type you actually get when calling `this.byId(...)` in a controller! +Don't forget using the specific event types like e.g. `Route$PatternMatchedEvent` for routing events. + +#### Casting Example + +BAD: +```ts +(this.byId("form") as unknown as {setVisible: (v: boolean) => void}).setVisible(false); +``` + +GOOD: + +```ts +import SimpleForm from "sap/ui/layout/form/SimpleForm"; +(this.byId("form") as SimpleForm).setVisible(false); +``` + +### Create shared type definitions + +Many type definitions you create are useful in different files. Create those in a central location like a file in in `src/types/`. + + +## Project Setup Conversion + +### 1. package.json +You must add the following dev dependencies in the package.json file (very important) if they are not already present: + +{{dependencies}} + +However, if a dependency is already present in package.json, do not increase the major version number of it. +Do not remove existing dependencies, you must only add new configuration. Install the dependencies early to verify the types are found. + +**IMPORTANT**: In addition, you **MUST** also add the `@sapui5/types` (or `@openui5/types`) package in a version matching the UI5 project as dev dependency. Framework type and version can be found in ui5.yaml or using the `get_project_info` MCP tool. + +In addition, if (and ONLY if) dependencies or their versions changed, ensure (or tell the user) to execute npm install / yarn install (whatever is used in the project) to get the changed dependencies in the project. + +The `typescript-eslint` dependency is only relevant when the project already has an eslint setup (details are below). + +Also add the `"ts-typecheck": "tsc --noEmit"` script to `package.json`, so you and the developer can easily check for TypeScript errors. + +### 2. tsconfig.json + +Add a tsconfig.json file. Use the following sample as reference, but adapt to the needs of the current project, e.g. adapt the paths map: + +```json +{ + "compilerOptions": { + "target": "es2023", + "module": "es2022", + "moduleResolution": "node", + "skipLibCheck": true, + "allowJs": true, + "strict": true, + "strictNullChecks": false, + "strictPropertyInitialization": false, + "outDir": "./dist", + "rootDir": "./webapp", + "types": ["@sapui5/types", "@types/jquery", "@types/qunit"], + "paths": { + "com/myorg/myapp/*": ["./webapp/*"], + "unit/*": ["./webapp/test/unit/*"], + "integration/*": ["./webapp/test/integration/*"] + } + }, + "exclude": ["./webapp/test/e2e/**/*"], + "include": ["./webapp/**/*"] +} +``` + +### 3. ui5.yaml + +Update the ui5.yaml file to use the `ui5-tooling-transpile-task` and `ui5-tooling-transpile-middleware` and ensure that at least the following config is present: + +```yaml +builder: + customTasks: + - name: ui5-tooling-transpile-task + afterTask: replaceVersion +server: + customMiddleware: + - name: ui5-tooling-transpile-middleware + afterMiddleware: compression + - name: ui5-middleware-livereload + afterMiddleware: compression +``` + +Ensure that the generated ui5.yaml file is valid - avoid duplicate entries, each root configuration must only exist once. +If a configuration like `server` already exists, you must add to it instead of adding a second entry. + +### 4. Eslint configuration + +Only when the project has eslint set up, enhance the eslint configuration with TypeScript-specific parts. If eslint is not set up with dependency in package.json and an eslint config, then do nothing. +A complete eslint v9 compatible `eslint.config.mjs` file could e.g. look like this, but the actual content depends on the specific project, so you MUST adapt it! + +```js +import eslint from "@eslint/js"; +import globals from "globals"; +import tseslint from "typescript-eslint"; + +export default tseslint.config( + eslint.configs.recommended, + ...tseslint.configs.recommended, + ...tseslint.configs.recommendedTypeChecked, + { + languageOptions: { + globals: { + ...globals.browser, + sap: "readonly" + }, + ecmaVersion: 2023, + parserOptions: { + project: true, + tsconfigRootDir: import.meta.dirname + } + } + }, + { + ignores: ["eslint.config.mjs"] + } +); +``` + + +## Application Code Conversion + +### Step 1: Change proprietary UI5 class syntax to standard ES class syntax + +Every UI5 class definitions (`SuperClass.extend(...)`) must be converted to a standard JavaScript `class`. +The properties in the UI5 class configuration object (second parameter of `extend`) become members of the standard JavaScript class. +It is important to annotate the class with the namespace in a JSDoc comment, so the back transformation can re-add it. +The namespace is the part of the full package+class name (first parameter of `extend`) that precedes the class name. + +Before (example): + +```js +[... other code, e.g. loading the dependencies "App", "Controller" etc. ...] + +var App = Controller.extend("ui5tssampleapp.controller.App", { + onInit: function _onInit() { + // apply content density mode to root view + this.getView().addStyleClass(this.getOwnerComponent().getContentDensityClass()); + } +}); +``` + + +After (example, do not use this code verbatim): + +```js +[... other code, e.g. loading the dependencies "App", "Controller" etc. ...] + +/** +* @namespace ui5tssampleapp.controller +*/ +class App extends Controller { + public onInit() { + // apply content density mode to root view + this.getView().addStyleClass((this.getOwnerComponent()).getContentDensityClass()); + }; +}; +``` + + +### Step 2: Change to ECMAScript modules and imports + +TypeScript UI5 apps must use modern ES modules and imports. +Hence, convert all UI5 module definition and dependency loading calls (`sap.ui.require(...)`, `sap.ui.define(...)`) +to ES modules with imports (and in case of `sap.ui.define` a module export). + +In the above example, this looks as follows. + +Before: + +```js +sap.ui.define(["sap/ui/core/mvc/Controller"], function (Controller) { + /** + * @namespace ui5tssampleapp.controller + */ + class App extends Controller { + ... // as above + }; + + return App; +}); +``` + +After: + +```js +import Controller from "sap/ui/core/mvc/Controller"; + +/** +* @namespace ui5tssampleapp.controller +*/ +export default class App extends Controller { + ... // as above +}; +``` + +`sap.ui.require` shall be converted to just the imports and no export. +Avoid name clashes for the imported modules. + +> Hint: importing `sap/ui/core/Core` does not provide the class (like for most other UI5 modules), but the singleton instance of the UI5 Core. So the imported module can be used directly for methods like `byId(...)` instead of calls to `sap.ui.getCore()` which return the singleton in JavaScript. + +When `sap.ui.require` is used dynamically, e.g. `sap.ui.require(["sap/m/MessageBox"], function(MessageBox) { ... })` inside a method body, then convert this to a dynamic import like `import("sap/m/MessageBox").then((MessageBox) => { ... })`. + +### Step 3: Standard TypeScript Code Adaptations + +Apply your general knowledge about converting JavaScript code to TypeScript. In particular: + +- Add type information to method parameters and variables where needed. +- Add missing private member class variables (with type information) to the beginning of the class definition. (In JavaScript they are often created later on-the-fly during the lifetime of a class instance.) +- Convert conventional `function`s to arrow functions when `someFunction.bind(...)` is used because TypeScript does not seem to propagate the type of the bound "this" context into the function body. +- Define further types and structures needed within the code, if applicable. + +> IMPORTANT: whenever you use a UI5 type, e.g. for annotating a variable or method parameter/returntype, do NOT use the UI5 type with its global namespace (like `sap.m.Button` or `sap.ui.core.Popup`)! Instead, import this UI5 type from the respective module (like `sap/m/Button` or `sap/ui/core/Popup` - add an import if needed) and use the imported module. + +Example: + +Wrong: +```ts +const b: sap.m.Button; +function getPopup(): sap.ui.core.Popup { ... } +``` + +Correct: +```ts +import Button from "sap/m/Button"; +import Popup from "sap/ui/core/Popup"; + +const b: Button; +function getPopup(): Popup { ... } +``` + +Hint: use the actual UI5 control events, not browser events like `Event` or `MouseEvent`, in event handlers of UI5 controls. UI5 events are different. E.g. use the `Button$PressEvent` and `Button$PressEventParameters` from the `sap/m/Button` module when the `press` event of the `sap/m/Button` is handled. + +> Note: for any event XYZ of a UI5 control ABC, types like `ABC$XYZEvent` and `ABC$XYZEventParameters` are available! + + +Example: + +Before: + +```js +sap.ui.define(["./BaseController"], function (BaseController) { + return BaseController.extend("my.app.controller.Main", { + onPress: function(oEvent) { + const button = oEvent.getSource(); + }, + + onSelectionChange: function(oEvent) { + const items = oEvent.getParameter("selectedItems"); + } + }); +}); +``` + +After: + +```ts +import BaseController from "./BaseController"; +import Button from "sap/m/Button"; +import {Button$PressEvent} from "sap/m/Button"; +import {Table$RowSelectionChangeEvent} from "sap/ui/table/Table"; + +export default class Main extends BaseController { + onPress(oEvent: Button$PressEvent): void { + const button = oEvent.getSource() as Button; + } + + onRowSelectionChange(oEvent: Table$RowSelectionChangeEvent): void { + const selectedContext = oEvent.getParameter("rowContext"); + } +} +``` + + +Hint: use the most specific type which does provide all needed properties. Examples: +- Use specific types like `KeyboardEvent` or `MouseEvent`, not just `Event` for browser events. +- Use the `Button$PressEvent` from the `sap/m/Button` module, not the `sap/ui/base/Event`. +- The same is valid for all types, not only events. + + +### Step 4: Casts for Return Values of Generic Methods + +Generic getter methods like `document.getElementById(...)` or `someUI5Control.getModel()` or inside a controller `this.byId()` return the super-type of all possible types (in the examples `HTMLElement` and `sap.ui.model.Model` and `sap.ui.core.Element`) although in practice it will usually be a specific sub-type (e.g. an `HTMLAnchorElement` or a `sap.ui.model.odata.v4.ODataModel` or a `sap.m.Input`). + +In many cases you will have to cast the return value to the specific type to use it. The actual type can usually be derived from the context. If not, rather avoid the cast than guessing a wrong one. Also, do not cast to a superclass like `sap.ui.model.Model` when this is anyway the returned type. + +The same is valid for several UI5 methods, most prominently the following: +- core.byId() / view.byId() +- control.getBinding() +- ownerComponent.getModel() +- event.getSource() +- component.getRootControl() +- this.getOwnerComponent() + +This cast will sometimes also require an additional module import to make the type (like `ODataModel` above) known. + +In the app controller example above, this step would add an additional import of the app's component (called `AppComponent`), so within the `onInit` implementation the required typecast can be done. Without this typecast, the return type of `getOwnerComponent` would be a `sap.ui.core.Component`, which does not have the `getContentDensityClass` method defined in the app component. + +Before: +```js +import Controller from "sap/ui/core/mvc/Controller"; + +/** +* @namespace ui5tssampleapp.controller +*/ +export default class App extends Controller { + + public onInit() { + // apply content density mode to root view + this.getView().addStyleClass(this.getOwnerComponent().getContentDensityClass()); + }; + +}; +``` + +After: +```ts +import Controller from "sap/ui/core/mvc/Controller"; +import AppComponent from "../Component"; + +/** +* @namespace ui5tssampleapp.controller +*/ +export default class App extends Controller { + + public onInit() : void { + // apply content density mode to root view + this.getView().addStyleClass((this.getOwnerComponent() as AppComponent).getContentDensityClass()); + }; + +}; +``` + + +(Note: the "void" definition of the method return type is not strictly demanded by TypeScript, but is beneficial e.g. depending on the linting settings.) + + +### Step 5: Solving any Remaining Issues + +At this point, the number of remaining TypeScript errors should be vastly reduced. +If you clearly recognize some, fix them, but in case of doubt mention the last remaining issues to the developer. + + +## UI5 Control TypeScript Conversion Guidelines + +> *This section covers the conversion of UI5 custom controls from JavaScript to TypeScript. This applies both to single custom controls within applications and to control libraries.* + +Converting custom UI5 controls to TypeScript requires specific patterns in addition to the general TypeScript conversion (converting the proprietary UI5 class and syntax). + +### The Runtime-Generated Methods Problem (CRITICAL) + +**This is the most important aspect to understand.** + +UI5 generates getter/setter (and more) methods for all properties, aggregations, associations, and events at **runtime**. This means TypeScript cannot see these methods at development time, causing type errors. + +#### The Problem + +In a control with a `text` property defined in metadata: + +```typescript +static readonly metadata: MetadataOptions = { + properties: { + "text": "string" + } +}; +``` + +TypeScript will show errors when trying to use the generated methods: + +```typescript +rm.text(control.getText()); // ERROR: Property 'getText' does not exist on type 'MyControl' +``` + +Additionally, TypeScript doesn't know the constructor signature structure for initializing controls: + +```typescript +new MyControl("myId", {text: "Hello"}); // TypeScript doesn't know about the settings object structure +``` + +This affects: +- Property getters/setters: `getText()`, `setText()`, `bindText()` +- Aggregation methods: `addItem()`, `removeItem()`, `getItems()`, ... +- Association methods: `getLabel()`, `setLabel()` +- Event methods: `attachPress()`, `detachPress()`, `firePress()` +- Constructor settings object structure + +#### The Solution: @ui5/ts-interface-generator + +Install the interface generator tool as a dev dependency: + +```sh +npm install --save-dev @ui5/ts-interface-generator@{{ts-interface-generator-version}} +``` + +To make subsequent development easier, add a script like this to `package.json`: + +```json +{ + "scripts": { + "watch:controls": "npx @ui5/ts-interface-generator --watch" + } +} +``` + +NOTE: the tsconfig file related to the controls must be in the same directory in which the interface generator is launched. If you launch it in the root of your project and the tsconfig covering the TypeScript controls is in a subdirectory or has a different name than `tsconfig.json`, then call it like ` npx @ui5/ts-interface-generator --watch --config path/to/tsconfig.json`. + +After TypeScript conversion of all controls, run the generator once to generate the needed control interfaces: + +```bash +npm run watch:controls +``` + +This generates a `*.gen.d.ts` file (e.g., `MyControl.gen.d.ts`) containing TypeScript interfaces with all the runtime-generated methods. TypeScript merges these interfaces with the control class. + +These generated files should be committed to version control and never edited manually. + +#### Required Constructor Signatures (CRITICAL MANUAL STEP) + +After running the interface generator, you must manually copy the constructor signatures from the terminal output into the respective control class. + +The generator outputs something like: + +``` +===== BEGIN ===== +// The following three lines were generated and should remain as-is to make TypeScript aware of the constructor signatures +constructor(id?: string | $MyControlSettings); +constructor(id?: string, settings?: $MyControlSettings); +constructor(id?: string, settings?: $MyControlSettings) { super(id, settings); } +===== END ===== +``` + +**Copy these lines into the beginning of the class body**, before the metadata definition: + +```typescript +export default class MyControl extends Control { + // The following three lines were generated and should remain as-is to make TypeScript aware of the constructor signatures + constructor(id?: string | $MyControlSettings); + constructor(id?: string, settings?: $MyControlSettings); + constructor(id?: string, settings?: $MyControlSettings) { super(id, settings); } + + static readonly metadata: MetadataOptions = { + // ... + }; +} +``` + +### Control Metadata Typing + +The control metadata must be typed as `MetadataOptions`: + +```typescript +import type { MetadataOptions } from "sap/ui/core/Element"; + +export default class MyControl extends Control { + static readonly metadata: MetadataOptions = { + properties: { + "text": "string" + } + }; +} +``` + +**Important points:** +- Import `MetadataOptions` from `sap/ui/core/Element` for controls (or closest base class - also available for `sap/ui/core/Object`, `sap/ui/core/ManagedObject`, and `sap/ui/core/Component`) +- Use `import type` instead of `import` (design-time only, no runtime impact) +- `MetadataOptions` available since UI5 1.110; use `object` for earlier versions +- Typing prevents issues when inheriting from the control (inherited properties should not be repeated) + +### Namespace Annotation Required + +The `@namespace` JSDoc annotation is **required** for the transformer to generate correct UI5 class names: + +```typescript +/** + * @namespace ui5.typescript.helloworld.control + */ +export default class MyControl extends Control { + // ... +} +``` + +### Export Pattern + +**Must use `export default` immediately when defining the class**, otherwise ts-interface-generator will fail: + +```typescript +// CORRECT: +export default class MyControl extends Control { + // ... +} + +// WRONG - separate export: +class MyControl extends Control { + // ... +} +export default MyControl; +``` + +### Static Members for Metadata and Renderer + +Both metadata and renderer are defined as `static` class members: + +```typescript +import RenderManager from "sap/ui/core/RenderManager"; + +export default class MyControl extends Control { + static readonly metadata: MetadataOptions = { + properties: { + "text": "string" + } + }; + + static renderer = { + apiVersion: 2, + render: function (rm: RenderManager, control: MyControl): void { + rm.openStart("div", control); + rm.openEnd(); + rm.text(control.getText()); + rm.close("div"); + } + }; +} +``` + +The renderer can also be in a separate file (common in libraries) and should in this case stay separate when converting to TypeScript. + +The following JavaScript code: + +```javascript +sap.ui.define([ + "sap/ui/core/Control", + "./MyControlRenderer" +], function (Control, MyControlRenderer) { + "use strict"; + + return Control.extend("com.myorg.myapp.control.MyControl", { + ... + renderer: MyControlRenderer, + ... +``` + +is then converted to this TypeScript code: + +```typescript +import Control from "sap/ui/core/Control"; +import type { MetadataOptions } from "sap/ui/core/Element"; +import MyControlRenderer from "./MyControlRenderer"; + +/** + * @namespace com.myorg.myapp.control + */ +export default class MyControl extends Control { + ... + static renderer = MyControlRenderer; + ... +``` + +### Complete Control Example + +#### JavaScript (Before): + +```javascript +sap.ui.define([ + "sap/ui/core/Control", + "sap/ui/core/RenderManager" +], function (Control, RenderManager) { + "use strict"; + + var MyControl = Control.extend("ui5.typescript.helloworld.control.MyControl", { + metadata: { + properties: { + "text": "string" + }, + events: { + "press": {} + } + }, + + renderer: function (rm, control) { + rm.openStart("div", control); + rm.openEnd(); + rm.text(control.getText()); + rm.close("div"); + }, + + onclick: function() { + this.firePress(); + } + }); + + return MyControl; +}); +``` + +#### TypeScript (After): + +```typescript +import Control from "sap/ui/core/Control"; +import type { MetadataOptions } from "sap/ui/core/Element"; +import RenderManager from "sap/ui/core/RenderManager"; + +/** + * @namespace ui5.typescript.helloworld.control + */ +export default class MyControl extends Control { + // The following three lines were generated and should remain as-is to make TypeScript aware of the constructor signatures + constructor(id?: string | $MyControlSettings); + constructor(id?: string, settings?: $MyControlSettings); + constructor(id?: string, settings?: $MyControlSettings) { super(id, settings); } + + static readonly metadata: MetadataOptions = { + properties: { + "text": "string" + }, + events: { + "press": {} + } + }; + + static renderer = { + apiVersion: 2, + render: function (rm: RenderManager, control: MyControl): void { + rm.openStart("div", control); + rm.openEnd(); + rm.text(control.getText()); + rm.close("div"); + } + }; + + onclick(): void { + this.firePress(); + } +} +``` + +### Library-Specific Guidelines + +When converting entire control libraries (not just single controls in apps), additional steps are required: + +#### Library Module with Enums (CRITICAL to avoid XSS issues!) + +In `library.ts`, enums must be attached to the global library object for UI5 runtime compatibility: + +```typescript +import ObjectPath from "sap/base/util/ObjectPath"; + +// Define enum as TypeScript enum +export enum ExampleColor { + Red = "Red", + Green = "Green", + Blue = "Blue" +} + +// CRITICAL: Attach to global library object +const thisLib = ObjectPath.get("com.myorg.myui5lib") as {[key: string]: unknown}; +thisLib.ExampleColor = ExampleColor; +``` + +**Why this is critical for every enum in the library:** +- Control properties reference types as global names: `type: "com.myorg.myui5lib.ExampleColor"` +- UI5 runtime needs to find the enum via this global path +- Without this, UI5 cannot validate the property type +- This breaks type checking and can create XSS vulnerabilities as unchecked content can be written to HTML unexpectedly + + +#### Path Mapping in tsconfig.json + +For libraries, add path mappings for the library namespace: + +```json +{ + "compilerOptions": { + "paths": { + "com/myorg/mylib/*": ["./src/*"] + } + } +} +``` + +### Control Conversion Checklist + +When converting a control from JavaScript to TypeScript: + +1. Convert to ES6 class/module like regular UI5 modules +2. Add `@namespace` JSDoc annotation +3. Use `export default` **immediately** with class definition +4. Type metadata as `MetadataOptions` (import from appropriate base class) +5. Define metadata and renderer as `static` members +6. Install and run `@ui5/ts-interface-generator` +7. Copy constructor signatures from generator output into class +8. If in a library: manually attach enums to global library object +9. Preserve all JSDoc comments and documentation + + +## Test Conversion + +> *There are critical, non-obvious patterns for converting UI5 test code from JavaScript to TypeScript. Standard ES6 module/class conversions and renaming of files to `*.ts` also applies, like for regular application code and controls.* + +NOTE: The test code file related changes below (especially those for OPA tests) always apply when converting the tests to TypeScript, but the test setup and running (like in `testsuite.qunit.ts`) may depend on how exactly the tests are set up in the project. + +Test conversion should only happen once the rest of the application has been converted successfully. + +### Explicit QUnit Import Required + +Unlike JavaScript where QUnit is often used as a global, TypeScript requires explicit import: + +```typescript +import QUnit from "sap/ui/thirdparty/qunit-2"; +``` + +### Test Registration in testsuite.qunit.ts + +The testsuite configuration uses plain `export default` (no sap.ui.define wrapper): + +```typescript +export default { + name: "Testsuite for the com/myorg/myapp app", + defaults: { + page: "ui5://test-resources/...", + loader: { + paths: { + "com/myorg/myapp": "../", + "integration": "./integration", + "unit": "./unit" + } + } + }, + tests: { + "unit/unitTests": { title: "..." }, + "integration/opaTests": { title: "..." } + } +}; +``` + +### tsconfig.json Path Mappings for Tests + +**Essential**: Add path mappings and QUnit types (like in this sample, but adapt to the specific app which is converted): + +```json +{ + "compilerOptions": { + "types": ["@sapui5/types", "@types/qunit"], + "paths": { + "com/myorg/myapp/*": ["./webapp/*"], + "unit/*": ["./webapp/test/unit/*"], + "integration/*": ["./webapp/test/integration/*"] + } + } +} +``` + +### OPA Integration Tests - Fundamental Architecture Change + +JavaScript Pattern (OLD) - NOT USED IN TYPESCRIPT: + +```javascript +sap.ui.define(["sap/ui/test/opaQunit", "./pages/App"], (opaTest) => { + opaTest("should add an item", (Given, When, Then) => { + Given.iStartMyApp(); + When.onTheAppPage.iEnterText("test"); + Then.onTheAppPage.iShouldSeeItem("test"); + Then.iTeardownMyApp(); + }); +}); +``` + +TypeScript Pattern (NEW) - MUST BE USED: + +```typescript +import opaTest from "sap/ui/test/opaQunit"; +import AppPage from "./pages/AppPage"; +import QUnit from "sap/ui/thirdparty/qunit-2"; + +const onTheAppPage = new AppPage(); + +QUnit.module("Test Module"); + +opaTest("Should open dialog", function () { + onTheAppPage.iStartMyUIComponent({ + componentConfig: { name: "ui5.typescript.helloworld" } + }); + + onTheAppPage.iPressButton(); + onTheAppPage.iShouldSeeDialog(); + onTheAppPage.iTeardownMyApp(); +}); +``` + +Critical Rules: +1. **NO Given/When/Then parameters** in the opaTest callback +2. **Create page instances BEFORE tests**: `const onTheAppPage = new AppPage();` +3. **Call all methods directly on the page instance** (arrangements, actions, assertions, cleanup) + +### OPA Page Objects - Class-Based Only + +JavaScript uses createPageObjects() - DO NOT USE IN TYPESCRIPT + +TypeScript uses classes extending Opa5: + +```typescript +import Opa5 from "sap/ui/test/Opa5"; +import Press from "sap/ui/test/actions/Press"; + +const viewName = "ui5.typescript.helloworld.view.App"; + +export default class AppPage extends Opa5 { + iPressButton() { + return this.waitFor({ + id: "myButton", + viewName, + actions: new Press(), + errorMessage: "Did not find button" + }); + } + + iShouldSeeDialog() { + return this.waitFor({ + controlType: "sap.m.Dialog", + success: function () { + Opa5.assert.ok(true, "Dialog is open"); + }, + errorMessage: "Did not find dialog" + }); + } +} +``` + +Key Points: +- **NO createPageObjects()** - use ES6 class extending Opa5 +- **NO separation** between actions and assertions objects. They are regular class methods +- All lifecycle methods (iStartMyUIComponent, iTeardownMyApp) are inherited from Opa5 + +### Arrangements Pattern - Eliminated + +DO NOT create separate Arrangements classes in TypeScript. + +JavaScript often uses: +```javascript +sap.ui.define(["sap/ui/test/Opa5"], (Opa5) => { + return Opa5.extend("namespace.Startup", { + iStartMyApp() { this.iStartMyUIComponent({...}); } + }); +}); +``` + +TypeScript eliminates this - call `iStartMyUIComponent()` directly on the page instance in the journey. + +### OPA Test Registration (opaTests.qunit.ts) + +JavaScript typically has: +```javascript +sap.ui.define(["sap/ui/test/Opa5", "./arrangements/Startup", "./Journey1"], (Opa5, Startup) => { + Opa5.extendConfig({ arrangements: new Startup(), autoWait: true }); +}); +``` +TypeScript simplifies to: +```typescript +// import all your OPA tests here +import "integration/HelloJourney"; +``` + +No Opa5.extendConfig() needed, just import the journeys. + +### Code Coverage in case of using `ui5-test-runner` + +If (and only if!) the tests are set up using the `ui5-test-runner` tool, then the app must be started with a specific `ui5-coverage.yaml` configuration. Suitable `package.json` scripts may look like this: + +``` + "start-coverage": "ui5 serve --port 8080 --config ui5-coverage.yaml", + ... + "test-runner-coverage": "ui5-test-runner --url http://localhost:8080/test/testsuite.qunit.html --coverage -ccb 60 -ccf 100 -ccl 80 -ccs 80", + "test-ui5": "ui5-test-runner --start start-coverage --url http://localhost:8080/test/testsuite.qunit.html --coverage -ccb 60 -ccf 100 -ccl 80 -ccs 80", +``` + +The `test-runner-coverage` script expects `start-coverage` to be executed manually, `test-ui5` does it automatically. + +When adding such scripts, explain them to the user! + +The `ui5-coverage.yaml` file must configure the `ui5-tooling-transpile-middleware` like this by adding the `babelConfig`: + +```yaml +... +server: + customMiddleware: + - name: ui5-tooling-transpile-middleware + afterMiddleware: compression + configuration: + debug: true + babelConfig: + sourceMaps: true + ignore: + - "**/*.d.ts" + presets: + - - "@babel/preset-env" + - targets: defaults + - - transform-ui5 + - "@babel/preset-typescript" + plugins: + - istanbul + - name: ui5-middleware-livereload + afterMiddleware: compression +``` + +In this case, the `babel-plugin-istanbul` package must be added as dev dependency! (The other packages in the config are already required by `ui5-tooling-transpile`). diff --git a/src/registerTools.ts b/src/registerTools.ts index 72cc3c5..9adcd3c 100644 --- a/src/registerTools.ts +++ b/src/registerTools.ts @@ -11,6 +11,7 @@ import registerGetGuidelinesTool from "./tools/get_guidelines/index.js"; import registerGetVersionInfoTool from "./tools/get_version_info/index.js"; import registerGetIntegrationCardsGuidelinesTool from "./tools/get_integration_cards_guidelines/index.js"; import registerCreateIntegrationCardTool from "./tools/create_integration_card/index.js"; +import registerGetTypescriptConversionGuidelinesTool from "./tools/get_typescript_conversion_guidelines/index.js"; interface Options { useStructuredContentInResponse: boolean; @@ -54,6 +55,8 @@ export default function (server: McpServer, context: Context, options: Options) registerGetIntegrationCardsGuidelinesTool(registerTool, context); registerCreateIntegrationCardTool(registerTool, context); + + registerGetTypescriptConversionGuidelinesTool(registerTool, context); } export function _processResponse({content, structuredContent}: CallToolResult, options: Options) { diff --git a/src/tools/get_typescript_conversion_guidelines/index.ts b/src/tools/get_typescript_conversion_guidelines/index.ts new file mode 100644 index 0000000..47a9793 --- /dev/null +++ b/src/tools/get_typescript_conversion_guidelines/index.ts @@ -0,0 +1,32 @@ +import {getTypescriptConversionGuidelines} from "./typescriptConversionGuidelines.js"; +import {getLogger} from "@ui5/logger"; +import Context from "../../Context.js"; +import {RegisterTool} from "../../registerTools.js"; + +const log = getLogger("tools:get_typescript_conversion_guidelines"); + +export default function registerTool(registerTool: RegisterTool, _context: Context) { + registerTool("get_typescript_conversion_guidelines", { + description: "This tool MUST be called once before converting a " + + "UI5 (SAPUI5/OpenUI5) project from JavaScript to TypeScript. " + + "The instructions provided by this tool MUST be followed to ensure " + + "that the project setup and code is correctly converted.", + annotations: { + title: "Get TypeScript Conversion Guidelines", + readOnlyHint: true, + idempotentHint: true, + openWorldHint: false, + }, + }, async () => { + log.info("Retrieving TypeScript conversion guidelines..."); + const guidelines = await getTypescriptConversionGuidelines(); + return { + content: [ + { + type: "text", + text: guidelines, + }, + ], + }; + }); +} diff --git a/src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts b/src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts new file mode 100644 index 0000000..84aac51 --- /dev/null +++ b/src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts @@ -0,0 +1,78 @@ +import {readFile} from "node:fs/promises"; +import https from "https"; + +const typescriptConversionGuidelinesFileUrl = new URL( + "../../../resources/typescript_conversion_guidelines.md", + import.meta.url +); + +async function getLatestVersion(packageName: string): Promise { + return new Promise((resolve, reject) => { + https + .get(`https://registry.npmjs.org/${packageName}`, (res) => { + let data = ""; + + res.on("data", (chunk) => { + data += chunk; + }); + + res.on("end", () => { + try { + const json = JSON.parse(data) as {"dist-tags": {latest: string}}; + resolve(json["dist-tags"].latest); + } catch (_err) { + reject(new Error(`Failed to parse response for package ${packageName}`)); + } + }); + }) + .on("error", (err) => { + reject(err); + }); + }); +} + +async function getLatestVersions(dependencies: Record) { + const packageNames = Object.keys(dependencies); + const versionPromises = packageNames.map((packageName) => { + return getLatestVersion(packageName).catch((_error) => { + return dependencies[packageName]; + }); + }); + + const versions = await Promise.all(versionPromises); + + const latestVersions: Record = {}; + packageNames.forEach((packageName, index) => { + latestVersions[packageName] = versions[index]; + }); + + return latestVersions; +} + +const getLatestDevDependencies = async () => { + const packages = await getLatestVersions({ + "@ui5/cli": "^4", + "typescript": "^5", + "typescript-eslint": "^8", + "ui5-middleware-livereload": "^3", + "ui5-tooling-transpile": "^3", + }); + return { + devDependencies: packages, + }; +}; + +const getLatestTsInterfaceGeneratorVersion = async () => { + const version = await getLatestVersion("@ui5/ts-interface-generator").catch(() => { + return "^0"; + }); + return version; +}; + +export async function getTypescriptConversionGuidelines(): Promise { + let guidelines = await readFile(typescriptConversionGuidelinesFileUrl, {encoding: "utf-8"}); + guidelines = guidelines + .replace("{{dependencies}}", JSON.stringify(await getLatestDevDependencies(), null, 3)) + .replace("{{ts-interface-generator-version}}", await getLatestTsInterfaceGeneratorVersion()); + return guidelines; +} diff --git a/test/lib/tools/get_typescript_conversion_guidelines/index.ts b/test/lib/tools/get_typescript_conversion_guidelines/index.ts new file mode 100644 index 0000000..5c9004d --- /dev/null +++ b/test/lib/tools/get_typescript_conversion_guidelines/index.ts @@ -0,0 +1,151 @@ +import anyTest, {TestFn} from "ava"; +import sinonGlobal from "sinon"; +import esmock from "esmock"; +import {InvalidInputError} from "../../../../src/utils.js"; +import TestContext from "../../../utils/TestContext.js"; + +const test = anyTest as TestFn<{ + sinon: sinonGlobal.SinonSandbox; + registerToolCallback: sinonGlobal.SinonStub; + getTypescriptConversionGuidelinesStub: sinonGlobal.SinonStub; + registerGetTypescriptConversionGuidelinesTool: + typeof import("../../../../src/tools/get_typescript_conversion_guidelines/index.js").default; +}>; + +test.beforeEach(async (t) => { + t.context.sinon = sinonGlobal.createSandbox(); + + t.context.registerToolCallback = t.context.sinon.stub(); + + // Create stub for getTypescriptConversionGuidelines function + const getTypescriptConversionGuidelinesStub = t.context.sinon.stub(); + t.context.getTypescriptConversionGuidelinesStub = getTypescriptConversionGuidelinesStub; + + // Import the module with mocked dependencies + const {default: registerGetTypescriptConversionGuidelinesTool} = await esmock( + "../../../../src/tools/get_typescript_conversion_guidelines/index.js", + { + "../../../../src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.js": { + getTypescriptConversionGuidelines: getTypescriptConversionGuidelinesStub, + }, + } + ); + + t.context.registerGetTypescriptConversionGuidelinesTool = registerGetTypescriptConversionGuidelinesTool; +}); + +test.afterEach.always((t) => { + t.context.sinon.restore(); +}); + +test("registerGetTypescriptConversionGuidelinesTool registers the tool with correct parameters", (t) => { + const {registerToolCallback, registerGetTypescriptConversionGuidelinesTool} = t.context; + + registerGetTypescriptConversionGuidelinesTool(registerToolCallback, new TestContext()); + + t.true(registerToolCallback.calledOnce); + t.is(registerToolCallback.firstCall.args[0], "get_typescript_conversion_guidelines"); + + // Verify tool configuration + const toolConfig = registerToolCallback.firstCall.args[1]; + t.true(toolConfig?.description?.includes("TypeScript")); + t.is(toolConfig?.annotations?.title, "Get TypeScript Conversion Guidelines"); + t.true(toolConfig?.annotations?.readOnlyHint); + t.true(toolConfig?.annotations?.idempotentHint); + t.false(toolConfig?.annotations?.openWorldHint); +}); + +test("get_typescript_conversion_guidelines tool returns guidelines content on success", async (t) => { + const { + registerToolCallback, + registerGetTypescriptConversionGuidelinesTool, + getTypescriptConversionGuidelinesStub, + } = t.context; + + // Setup getTypescriptConversionGuidelines to return sample content + const sampleGuidelines = "# TypeScript Conversion Guidelines\n\nSample content with dependencies"; + getTypescriptConversionGuidelinesStub.resolves(sampleGuidelines); + + // Register the tool and capture the execute function + registerGetTypescriptConversionGuidelinesTool(registerToolCallback, new TestContext()); + const executeFunction = registerToolCallback.firstCall.args[2]; + + // Create a mock for the extra parameter + const mockExtra = { + signal: new AbortController().signal, + requestId: "test-request-id", + sendNotification: t.context.sinon.stub(), + sendRequest: t.context.sinon.stub(), + }; + + // Execute the tool + const result = await executeFunction({}, mockExtra); + + // Verify the result + t.deepEqual(result, { + content: [ + { + type: "text", + text: sampleGuidelines, + }, + ], + }); +}); + +test("get_typescript_conversion_guidelines tool handles errors correctly", async (t) => { + const { + registerToolCallback, + registerGetTypescriptConversionGuidelinesTool, + getTypescriptConversionGuidelinesStub, + } = t.context; + + // Setup getTypescriptConversionGuidelines to throw an error + const errorMessage = "Failed to read guidelines file"; + getTypescriptConversionGuidelinesStub.rejects(new Error(errorMessage)); + + // Register the tool and capture the execute function + registerGetTypescriptConversionGuidelinesTool(registerToolCallback, new TestContext()); + const executeFunction = registerToolCallback.firstCall.args[2]; + + // Create a mock for the extra parameter + const mockExtra = { + signal: new AbortController().signal, + requestId: "test-request-id", + sendNotification: t.context.sinon.stub(), + sendRequest: t.context.sinon.stub(), + }; + + // Execute the tool and expect it to throw + await t.throwsAsync(async () => { + await executeFunction({}, mockExtra); + }, {message: errorMessage}); +}); + +test("get_typescript_conversion_guidelines tool passes through SoftError", async (t) => { + const { + registerToolCallback, + registerGetTypescriptConversionGuidelinesTool, + getTypescriptConversionGuidelinesStub, + } = t.context; + + // Setup getTypescriptConversionGuidelines to throw a SoftError + const errorMessage = "Soft error occurred"; + getTypescriptConversionGuidelinesStub.rejects(new InvalidInputError(errorMessage)); + + // Register the tool and capture the execute function + registerGetTypescriptConversionGuidelinesTool(registerToolCallback, new TestContext()); + const executeFunction = registerToolCallback.firstCall.args[2]; + + // Create a mock for the extra parameter + const mockExtra = { + signal: new AbortController().signal, + requestId: "test-request-id", + sendNotification: t.context.sinon.stub(), + sendRequest: t.context.sinon.stub(), + }; + + // Execute the tool and expect it to throw the same SoftError + await t.throwsAsync(async () => { + await executeFunction({}, mockExtra); + }, {message: errorMessage, instanceOf: InvalidInputError}); +}); diff --git a/test/lib/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts b/test/lib/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts new file mode 100644 index 0000000..892de74 --- /dev/null +++ b/test/lib/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts @@ -0,0 +1,143 @@ +import anyTest, {TestFn} from "ava"; +import sinonGlobal from "sinon"; +import esmock from "esmock"; +import {EventEmitter} from "events"; + +const fakeGuidelines = "# TypeScript Conversion Guidelines\n\nDependencies: {{dependencies}}"; +const fakeNpmResponse = { + "dist-tags": { + latest: "1.0.0", + }, +}; + +async function getMockedModule(httpsStub?: sinonGlobal.SinonStub) { + const readFileStub = sinonGlobal.stub().resolves(fakeGuidelines); + + const mockHttpsStub = httpsStub ?? sinonGlobal.stub().returns({ + on: sinonGlobal.stub().returnsThis(), + }); + + const {getTypescriptConversionGuidelines} = await esmock( + "../../../../src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.ts", + { + "node:fs/promises": { + readFile: readFileStub, + }, + "node:https": { + default: { + get: mockHttpsStub, + }, + }, + } + ); + return {getTypescriptConversionGuidelines, readFileStub, httpsStub: mockHttpsStub}; +} + +const test = anyTest as TestFn<{ + sinon: sinonGlobal.SinonSandbox; + getTypescriptConversionGuidelines: typeof import( + "../../../../src/tools/get_typescript_conversion_guidelines/typescriptConversionGuidelines.js" + ).getTypescriptConversionGuidelines; + readFileStub: sinonGlobal.SinonStub; + httpsStub: sinonGlobal.SinonStub; +}>; + +test.beforeEach((t) => { + t.context.sinon = sinonGlobal.createSandbox(); +}); + +test.afterEach.always((t) => { + t.context.sinon.restore(); +}); + +test("getTypescriptConversionGuidelines reads file and replaces dependencies", async (t) => { + // Create a mock HTTPS response that returns successful npm registry data + const httpsStub = t.context.sinon.stub(); + + const mockResponse = new EventEmitter(); + httpsStub.callsFake((url: string, callback: (res: EventEmitter) => void) => { + // Simulate async response + setImmediate(() => { + callback(mockResponse); + setImmediate(() => { + mockResponse.emit("data", JSON.stringify(fakeNpmResponse)); + mockResponse.emit("end"); + }); + }); + + return { + on: t.context.sinon.stub().returnsThis(), + }; + }); + + const {getTypescriptConversionGuidelines, readFileStub} = await getMockedModule(httpsStub); + + const result = await getTypescriptConversionGuidelines(); + + // Verify file was read + t.true(readFileStub.calledOnce); + const callArg = readFileStub.firstCall.args[0]; + t.true( + callArg.toString().includes("typescript_conversion_guidelines.md"), + "readFile should be called with correct file path" + ); + + // Verify dependencies placeholder was replaced + t.false(result.includes("{{dependencies}}"), "Placeholder should be replaced"); + t.true(result.includes("devDependencies"), "Should contain devDependencies"); + t.true(result.includes("@ui5/cli"), "Should contain @ui5/cli package"); + t.true(result.includes("typescript"), "Should contain typescript package"); + t.true(result.includes("ui5-tooling-transpile"), "Should contain ui5-tooling-transpile package"); +}); + +test("getTypescriptConversionGuidelines handles npm registry errors gracefully", async (t) => { + // Create a mock HTTPS response that fails + const httpsStub = t.context.sinon.stub(); + + httpsStub.callsFake((_url: string, _callback: (res: EventEmitter) => void) => { + const mockRequest = new EventEmitter(); + + // Simulate error + setImmediate(() => { + mockRequest.emit("error", new Error("Network error")); + }); + + return mockRequest; + }); + + const {getTypescriptConversionGuidelines} = await getMockedModule(httpsStub); + + // Should not throw, but fall back to default versions + const result = await getTypescriptConversionGuidelines(); + + t.false(result.includes("{{dependencies}}"), "Placeholder should be replaced even on error"); + t.true(result.includes("devDependencies"), "Should contain devDependencies with fallback versions"); +}); + +test("getTypescriptConversionGuidelines handles malformed npm response", async (t) => { + // Create a mock HTTPS response with invalid JSON + const httpsStub = t.context.sinon.stub(); + + const mockResponse = new EventEmitter(); + httpsStub.callsFake((_url: string, callback: (res: EventEmitter) => void) => { + setImmediate(() => { + callback(mockResponse); + setImmediate(() => { + mockResponse.emit("data", "invalid json"); + mockResponse.emit("end"); + }); + }); + + return { + on: t.context.sinon.stub().returnsThis(), + }; + }); + + const {getTypescriptConversionGuidelines} = await getMockedModule(httpsStub); + + // Should not throw, but fall back to default versions + const result = await getTypescriptConversionGuidelines(); + + t.false(result.includes("{{dependencies}}"), "Placeholder should be replaced even with invalid response"); + t.true(result.includes("devDependencies"), "Should contain devDependencies with fallback versions"); +});