[deprecations] Remove/update core-api deprecations targeted for removal by 8.8 #147723
Conversation
| @@ -32,6 +32,7 @@ export type SavedObjectAttribute = SavedObjectAttributeSingle | SavedObjectAttri | |||
| * | |||
| * @public | |||
| * @deprecated This type reduces the type safety of your code. Create an interface for your specific saved object type or use `unknown` instead. | |||
| * @removeBy 9.0.0 | |||
There was a problem hiding this comment.
I added a removeBy version to encourage type-owners to implement their interfaces. If there is a 9.0.0 version, we'll probably have a bunch more deprecations to worry about and shouldn't be carrying existing ones over to a new major.
src/plugins/dashboard/server/dashboard_saved_object/migrations/migrate_to_730/migrations_730.ts
Show resolved
Hide resolved
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-core (Team:Core) |
|
@elasticmachine merge upstream |
packages/core/application/core-application-browser/src/app_mount.ts
Outdated
Show resolved
Hide resolved
| * @deprecated Use {@link AppMountParameters.history} instead. | ||
| * @removeBy 8.8.0 | ||
| * |
There was a problem hiding this comment.
Was this misplaced? Can't we remove appBasePath? Is it not really deprecated?
There was a problem hiding this comment.
Um, at first I thought some teams were blocked on using AppMountParams.history over AppMountParams.appBasePath because they couldn't use ScopedHistory.bock over onAppLeave for back button navigation in the browser.
Now though, I'm not so sure.
We have 2 options:
- Leave the deprecation in place until teams still using
appBasePathmigrate away from it
e.g src/plugins/kibana_overview/public/application.tsx: or - Carry on supporting
appBasePathsince we're not deprecatingonAppLeaveorAppLeaveHandler.
@pgayvallet could you recommend the best path forward please?
@afharo For this PR, I'll add the deprecation warning back in unless the recommendation is to continue supporting it.
There was a problem hiding this comment.
IIRC appBasePath exposition isn't directly related to the ScopedHistory.block thingy. It's exposed because applications based on the legacy (Hash) router needs to know about it to instantiate their own router. I think nothing would block them from migrating to the new (HTML5) router.
Q/A:
should it be deprecated? ihmo yes.
would it make sense to delete it? ihmo yes.
can we delete it? no, as other teams are still using it...
can we easily adapt the code ourselves to delete it? no, migrating from one router to the other is all but trivial, and multiple apps are still using it IIRC
so, what should we do? not sure. Removing the deprecation may be fine, idk.
There was a problem hiding this comment.
We have a bit of time to see if teams have the capacity to migrate their routers and don't need to make a decision right now.
If there's no actual harm in keeping appBasePath around then we can treat it in the same way as onAppLeave: remove the deprecation warning even though it's not ideal.
| * @deprecated Use `warn` instead. | ||
| * @removeBy 8.8.0 | ||
| */ | ||
| warning: (msg: string) => void; |
There was a problem hiding this comment.
We should also adapt the implementation accordingly (MigrationLogger in
There was a problem hiding this comment.
Good catch, thanks!
For the record, the discrepancy in log level type between 'SavedObjectsMigrationLogger' and 'MigrationLogLevel':
export interface SavedObjectsMigrationLogger {
debug: (msg: string) => void;
info: (msg: string) => void;
warn: (msg: string) => void;
error: <Meta extends LogMeta = LogMeta>(msg: string, meta: Meta) => void;
}
vs
/** @internal */
export type MigrationLogLevel = 'error' | 'info' | 'warning';
Is intentional. The log level gets cast from warning to warn in logStateTransition:
case 'warning':
return logger.warn(logMessagePrefix + message);
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
merge conflict between base and head |
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Unknown metric groupsAPI count
References to deprecated APIs
History
To update your PR or re-run it, just comment with: |
Core API's need to be robust and reliable for feature development, even more so as we prepare for backward compatibility between versions.
A number of deprecated API's are currently marked for removal by 8.8 and this PR removes those that can safely be removed and removes the deprecation from those that we will not be removing.
Part of 142915
Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers
Release note:
SavedObjectsMigrationLogger.warningtargeted for removal by 8.8AppMountParameters.onAppLeave.AppLeaveHandlerandOpsProcessMetrics