Skip to content
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

fix: support wrapped controller extension assignments #132

Merged
merged 2 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/plugin/__test__/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1875,6 +1875,24 @@ exports[`typescript ts-class-controller-extension-usage-new.ts 1`] = `
});"
`;

exports[`typescript ts-class-controller-extension-wrapped.ts 1`] = `
"sap.ui.define(["sap/ui/core/mvc/Controller", "sap/ui/core/mvc/ControllerExtension", "sap/fe/core/controllerextensions/Routing"], function (Controller, ControllerExtension, Routing) {
"use strict";

const cov_1uvvg22e7l = () => {
return {
"s": {}
};
};
const MyExtendedController = Controller.extend("test.controller.MyExtendedController", {
routing3: Routing.override({}),
akudev marked this conversation as resolved.
Show resolved Hide resolved
routing2: Routing.override({}),
routing: Routing
});
return MyExtendedController;
});"
`;

exports[`typescript ts-class-param-props.ts 1`] = `
"sap.ui.define(["sap/Class"], function (SAPClass) {
"use strict";
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import Controller from "sap/ui/core/mvc/Controller";
import ControllerExtension from "sap/ui/core/mvc/ControllerExtension";
import Routing from "sap/fe/core/controllerextensions/Routing";

const cov_1uvvg22e7l = () => { return { "s": {} }; }; // dummy coverage function

/**
* @namespace test.controller
*/
export default class MyExtendedController extends Controller {

// code could already be instrumented, e.g. for code coverage by istanbul, and look like this:
//this.routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({ … })));
routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing));
routing2 = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})));
routing3 = (cov_1uvvg22e7l().s[5]++, cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({})));
}
73 changes: 47 additions & 26 deletions packages/plugin/src/classes/helpers/classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,37 +222,58 @@ export function convertClassToUI5Extend(
// @transformControllerExtension marker, because it is not a type assignment. In the resulting code, the
// "ControllerExtension.use(...)" part should be removed and the content of the brackets should be assigned
// directly to the member property.
if (t.isCallExpression(member.value)) {
const callee = member.value.callee;
if (
t.isMemberExpression(callee) &&
t.isIdentifier(callee.object) &&
t.isIdentifier(callee.property) &&
callee.property.name === "use" // we are looking for "ControllerExtension.use(...)"
) {
const importDeclaration = getImportDeclaration(
memberPath?.hub?.file?.opts?.filename,
callee?.object?.name // usually, but not necessarily always: "ControllerExtension"...
);
// ...hence we rather look at the imported module name to be sure
if (
t.isCallExpression(member.value) ||
t.isSequenceExpression(member.value)
) {
let callExpression = member.value;

// code instrumentation sometimes wraps it like:
// this.routing = (cov_1uvvg22e7l().s[5]++, ControllerExtension.use(Routing.override({ … })));
if (t.isSequenceExpression(member.value)) {
// iterate through the expressions in the sequence
for (const expr of member.value.expressions) {
if (t.isCallExpression(expr)) {
callExpression = expr;
break;
}
}
}

akudev marked this conversation as resolved.
Show resolved Hide resolved
if (t.isCallExpression(callExpression)) {
const callee = callExpression.callee;
if (
importDeclaration?.source?.value ===
"sap/ui/core/mvc/ControllerExtension"
t.isMemberExpression(callee) &&
t.isIdentifier(callee.object) &&
t.isIdentifier(callee.property) &&
callee.property.name === "use" // we are looking for "ControllerExtension.use(...)"
) {
const importDeclaration = getImportDeclaration(
memberPath?.hub?.file?.opts?.filename,
callee?.object?.name // usually, but not necessarily always: "ControllerExtension"...
);
// ...hence we rather look at the imported module name to be sure
if (
!member.value.arguments ||
member.value.arguments.length !== 1
importDeclaration?.source?.value ===
"sap/ui/core/mvc/ControllerExtension"
) {
// exactly one argument must be there
throw memberPath.buildCodeFrameError(
`ControllerExtension.use() must be called with exactly one argument but has ${
member.value.arguments ? member.value.arguments.length : 0
}`
);
if (
!callExpression.arguments ||
callExpression.arguments.length !== 1
) {
// exactly one argument must be there
throw memberPath.buildCodeFrameError(
`ControllerExtension.use() must be called with exactly one argument but has ${
callExpression.arguments
? callExpression.arguments.length
: 0
}`
);
}
member.value = callExpression.arguments[0];
extendProps.unshift(buildObjectProperty(member)); // add it to the properties of the extend() config object
continue; // prevent the member from also being added to the constructor
}
member.value = member.value.arguments[0];
extendProps.unshift(buildObjectProperty(member)); // add it to the properties of the extend() config object
continue; // prevent the member from also being added to the constructor
}
}
}
Expand Down
Loading