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

Add generic type for watchdog data. #13697

Closed
wants to merge 1 commit into from

Conversation

mremiszewski
Copy link
Contributor

Suggested merge commit message (convention)

Feature (watchdog): Add a generic type for watchdog data. See #13541.


Additional information

For example – encountered issues, assumptions you had to make, other affected tickets, etc.

Comment on lines -197 to +208
public getItem( itemId: string ): unknown {
const watchdog = this._getWatchdog( itemId );
public getItem( itemId: string ): unknown | undefined {
try {
const watchdog = this._getWatchdog( itemId );

return watchdog._item;
return watchdog._item;
}
catch {
return undefined;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if this change is valid, but docs say it should return undefined if not found but it was throwing error instead. If it's valid change, then tests should be adjusted.

@niegowski
Copy link
Contributor

I think we should abandon this idea for now. This looks complicated and has a very low discoverability. This is not blocking integrations, only shifts responsibility for maintaining either HTML element or editor data used to initialize an editor.

@CKEditorBot
Copy link
Collaborator

There has been no activity on this PR for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the contribution, leave a comment or reaction under this PR.

@CKEditorBot
Copy link
Collaborator

We've closed your PR due to inactivity. While time has passed, the core of your contribution might still be relevant. If you're able, consider reopening a similar PR.

@CKEditorBot CKEditorBot added resolution:expired This issue was closed due to lack of feedback. and removed status:stale labels Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
resolution:expired This issue was closed due to lack of feedback.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants