Skip to content

feat(dts-generator): add TypedJSONModel #516

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

martinbroede
Copy link

Add the type definitions for the TypedJSONModel and adapt the generator to insert them to the core d.ts file.

Related to #509 – thanks for your submission, @akudev !
I added tests for TypedJSONModel, which can be executed separately via npm run test:typed-json-model, so they can be added to CI when appropriate.

Copy link

cla-assistant bot commented Jul 29, 2025

CLA assistant check
All committers have signed the CLA.

@akudev
Copy link
Member

akudev commented Jul 29, 2025

rebase now against the primary error in node 18 and update the expected test result in openui5-snapshot-test with "npm run re-generate"

@akudev
Copy link
Member

akudev commented Jul 29, 2025

Related to #509 – thanks for your submission, @akudev !

Ah, that was just putting your code to the right places - thanks a lot for the implementation, it's great to get this in!

In addition to the comments above, as I told you, please document what to do when a fix is needed, esp. how to update the code in the dts-generator. My prior mail had the description how I split type definitions and implementation. For a small fix or extension you'd probably just apply the changes to what we already have in the dts-generator and in the OpenUI5 repo, not do the full splitting exercise again.

Also, we need some documentation for users of the TJM. We only have the JSDoc in the types themselves (good but hard to discover when you don't know the feature) and the sample app deep inside this repo. If you write up some markdown, we can find a good place. E.g. in the gh-pages branch of this repo, where we can get it in immediately (and where you can develop the docu as PR).
Ultimately, however, it should be in the UI5 documentation, ideally in the same release when we enable it and when it would be noted in the "What's new section" (need to trigger this as well!). Probably as a section or sub-page of https://sdk.openui5.org/topic/96804e3315ff440aa0a50fd290805116#loio96804e3315ff440aa0a50fd290805116
Doesn't need to be much.

That place would also be a good one to mention the limitations. Including the relatively esoteric technical ones within the model you already described, but more importantly the real-life ones outside the TJM itself which are more likely to be seen by users, like that in XMLView bindings there is of course no type safety provided by using a typed model (so you can still bind to non-existing properties) etc.

Copy link
Member

@akudev akudev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, this looks good to me now. Only the version dependency of adding the TJM types to the dts file is missing.

@codeworrior , @petermuessig if you want to have a look as well...

@petermuessig
Copy link
Contributor

petermuessig commented Aug 6, 2025

AFAICS and besides the already raised comments (Andreas, Frank) it looks good. Can't wait to see it live...

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

Successfully merging this pull request may close these issues.

3 participants