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

chore: fix svelte 5 deprecation warning #9331

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

deboer-tim
Copy link
Contributor

What does this PR do?

Replaces code to remove deprecation warning.

Screenshot / video of UI

N/A

What issues does this PR fix or reference?

Fixes #9313.

How to test this PR?

PR checks.

@deboer-tim deboer-tim requested review from benoitf and a team as code owners October 10, 2024 20:02
@deboer-tim deboer-tim requested review from jeffmaury, axel7083 and SoniaSandler and removed request for a team October 10, 2024 20:02
@benoitf
Copy link
Collaborator

benoitf commented Oct 10, 2024

there is a typecheck failure then

Copy link
Contributor

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

Seems other change is required

@axel7083
Copy link
Contributor

Weirdly enough we need to remove the following lines

Note sure if they are important ? But removing them fix the svelte-check on my machine

@deboer-tim
Copy link
Contributor Author

I haven't tried yet, but I'd expect removing that would break reactivity for actions, e.g. if you click start, the icon button on the left wouldn't change.

@axel7083
Copy link
Contributor

I haven't tried yet, but I'd expect removing that would break reactivity for actions, e.g. if you click start, the icon button on the left wouldn't change.

It does, I've tried it, and it seems to work fine

@benoitf
Copy link
Collaborator

benoitf commented Oct 17, 2024

setting to draft as it's not passing pr checks

@benoitf benoitf marked this pull request as draft October 17, 2024 21:28
@deboer-tim
Copy link
Contributor Author

Sorry for the delay. Rebased and included on:update removal, moving back to review. I wanted a chance to thoroughly test, and I could not break it. I concur with @axel7083, on:update is no longer required, svelte 5 ftw.

@deboer-tim deboer-tim marked this pull request as ready for review October 21, 2024 14:31
Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

I tested running, starting, stopping, removing containers, the table is working on my side.

Also deleting images works, the table update properly

@deboer-tim
Copy link
Contributor Author

@jeffmaury please re-review

Copy link
Contributor

@cdrage cdrage left a comment

Choose a reason for hiding this comment

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

Checked with Pods and Containers, it all updated fine to me 👍

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.

Bug(ui-library) Column typing is not compatible with Svelte 5
5 participants