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

Deprecate some replaced metadata methods and improve metadata API #7783

Merged
merged 76 commits into from
Mar 25, 2025

Conversation

rschili
Copy link
Contributor

@rschili rschili commented Feb 27, 2025

I replaced some callers with the new calls, and ignored the deprecation warning on others. Eventually we will want to replace them all.

Deprecated methods/types
Common package:

  • EntityClassMetadata
  • EntityClassMetadataProps
  • CustomAttribute
  • PropertyMetaData
  • PropertyMetaDataProps

Backend package

  • Element.getClassMetaData()
  • Entity.forEachProperty()
  • IModelDb.classMetaDataRegistry
  • IModelDb.getMetaData
  • IModelDb.tryGetMetaData
  • IModelDb.forEachMetaData()
  • MetaDataRegistry

Also made the following (breaking) changes to ecschema-metadata:

  • getSchemaItems and getProperties now always consistenty return Iterable. Before they returned IterableIterator or Arrays inconsistently.
  • getProperties had a "clearCache" flag, that is gone. Cache is now implicitly cleared for the local class. For changes to the base classes, an internal "cleanCache" method has been added.
  • .properties on Class is gone. It returned local properties or undefined. getProperties now takes an optional boolean flag which makes it return only local properties.

See Nextversion.md for a more comprehensive list of changes.
There are several new shortcuts and helper method that make transitioning easier.

Parent: https://github.com/iTwin/itwinjs-backlog/issues/1156
Part of: https://github.com/iTwin/itwinjs-backlog/issues/1415

@rschili rschili added the deprecation For proposed deprecations to the public API label Feb 27, 2025
@rschili rschili added this to the iTwin.js 5.0 milestone Feb 27, 2025
@rschili rschili marked this pull request as ready for review March 4, 2025 10:43
@rschili rschili requested review from a team, ColinKerr and diegoalexdiaz as code owners March 4, 2025 10:43
Copy link
Member

@grigasp grigasp left a comment

Choose a reason for hiding this comment

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

I don't want to have eslint-disable-next-line @typescript-eslint/no-deprecated in our owned code. I'll try to find some time to update it this week.

@rschili
Copy link
Contributor Author

rschili commented Mar 4, 2025

@grigasp

I don't want to have eslint-disable-next-line @typescript-eslint/no-deprecated in our owned code. I'll try to find some time to update it this week.

Hey, that's a fair point. I want to improve the messages on those deprecation warnings to point to the replacement. But I realized that our replacements are much more verbose which I don't like. I will do a follow up story to provide better drop-in replacements, shortcuts, or make other API improvements to ease the transition.
So for now I suggest we keep those deprecation ignores. It will probably be easier to switch in one or two weeks. Of course I won't keep you from doing it if you still prefer

Copy link
Member

@grigasp grigasp left a comment

Choose a reason for hiding this comment

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

reviewed changes under presentation dir & NextVersion.md.

Copy link
Contributor

@christophermlawson christophermlawson left a comment

Choose a reason for hiding this comment

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

Looks good. Just that one typo.

Copy link
Contributor

mergify bot commented Mar 21, 2025

This pull request is now in conflicts. Could you fix it @rschili? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

@rschili
Copy link
Contributor Author

rschili commented Mar 24, 2025

Hey @aruniverse @hl662 @christophermlawson could you give this another look?
There is one open comment by Nam about the new API being public. We will want to mark it stable for one release, but we cannot wait until that mechanism becomes available.
I can change them to beta if that's preferred, I don't think it makes a real difference. The APIs used under the hood are well tested and stable.

Thanks

Copy link
Contributor

mergify bot commented Mar 24, 2025

This pull request is now in conflicts. Could you fix it @rschili? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

@rschili rschili merged commit be6d5eb into master Mar 25, 2025
16 checks passed
@rschili rschili deleted the rob/deprecate-metadata branch March 25, 2025 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecation For proposed deprecations to the public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants