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

TS Types for different UI5 versions (Question) #7

Closed
lemaiwo opened this issue May 26, 2021 · 3 comments
Closed

TS Types for different UI5 versions (Question) #7

lemaiwo opened this issue May 26, 2021 · 3 comments
Assignees

Comments

@lemaiwo
Copy link

lemaiwo commented May 26, 2021

Hi all,

I noticed some differences in the TS types for other UI5 versions. Can we generate the types for other versions as well?

One more thing, I noticed that the names of the controllers also contain "Controller" at the end while this is not added in the controllername defined in the view. Does the framework excludes "Controller" when connecting the view to the controller?

Thank you in advance!

Kr, Wouter

@akudev
Copy link
Contributor

akudev commented Jun 8, 2021

Hi Wouter,

you mean types for previous UI5 versions? There is definitely a difference - we have invested quite some time to improve the type definitions and the new ones are for writing UI5 code that uses ES modules. I'm unsure whether we can provide the "new" type definitions also for older UI5 versions. We have also done a lot of fixes/improvements in UI5 itself to make the generated type definitions error-free and "good". We can't downport all those changes. We could look into what happens when we just run the generator on an older version of UI5 and how the result looks. There will be errors, but most of them don't actually prevent the usage of the definition files (those few that do can probably be fixed). So maybe an unsupported one-time drop for select long-term-maintenance releases MIGHT be an option to look into after UI5con, but more than that most probably not.

In your second question, you mean the last part of

const AppController = Controller.extend("ui5.typescript.helloworld.controller.AppController", {

, right? It should just be controller.App. This looks fishy indeed.
It will still work because the file name matches the controller name in the View, so the correct file is loaded. And I guess once the controller was loaded, nobody cares about the name it gave to itself. But for instantiating manually one would have the wrong longer name with appended "Controller":

image

It seems like the code transformer uses the class name given here:
https://github.com/SAP-samples/ui5-cap-event-app/blob/typescript/packages/ui-form/src/controller/App.controller.ts#L7

Probably we should rename the class name here to avoid this odd situation. But one has to take care to avoid name clashes in such a case, e.g. with the sap.m.App control type. Let's keep this issue report open to follow up.

Regards
Andreas

@akudev akudev self-assigned this Jun 8, 2021
@lemaiwo
Copy link
Author

lemaiwo commented Jun 9, 2021

Thank you Andreas!

The reason I'm asking about the different TS types for different UI5 versions is because I found a difference for the "navTo" function.

We are using UI5 1.71 which has different params for the "navTo" function:
https://ui5.sap.com/1.71.38/#/api/sap.ui.core.routing.Router/methods/navTo
version 1.90:
https://ui5.sap.com/#/api/sap.ui.core.routing.Router%23methods/navTo

In vscode, this gives me an error on the "navTo" function while I'm using it in the correct way.
image

I guess it wouldn't harm to generate it for each version to cover these kind of differences.

Regarding the name, that means that renaming the names in TS without "Controller" would be fine?

Kr, Wouter

@akudev
Copy link
Contributor

akudev commented Dec 29, 2021

Hi,
just noticed this now, way later, but we talked in the meantime, anyway. :-)
For sake of completeness: yes, renaming without "Controller" would be fine.
And no, we can't generate the TS interfaces for older versions of UI5, as there are also changes in the UI5 code required, not only in the generator, to get to a good result. Hence it would be dishonest to keep this issue report open, pretending that it will be done, sorry.
Once we - finally - publish the new version of the dts generator, anyone can try building the definitions for older versions on their own, but they will contain issues.

Regards
Andreas

@akudev akudev closed this as completed Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants