chore: migrate more createEventDispatcher calls #7187
Conversation
Deploying with
|
| Latest commit: |
91361c3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5a17f3b8.immich.pages.dev |
| Branch Preview URL: | https://migrate-more.immich.pages.dev |
benmccann
force-pushed
the
migrate-more
branch
4 times, most recently
from
0e517fe
to
c78c951
Compare
benmccann
force-pushed
the
migrate-more
branch
from
c78c951
to
e3c9728
Compare
| on:outclick={() => !ignoreClickOutside && dispatch('close')} | ||
| on:escape={() => dispatch('escape')} | ||
| on:outclick={() => !ignoreClickOutside && onClose && onClose()} | ||
| on:escape={() => onEscape && onEscape()} |
There was a problem hiding this comment.
| on:escape={() => onEscape && onEscape()} | |
| on:escape={() => onEscape?.()} |
There was a problem hiding this comment.
FYI: You can use the ?. null-ish chaining operator even when invoking functions
| on:newAlbum={({ detail }) => handleAddToNewAlbum(detail)} | ||
| on:album={({ detail }) => handleAddToAlbum(detail)} | ||
| on:close={() => (isShowAlbumPicker = false)} | ||
| onNewAlbum={handleAddToNewAlbum} |
There was a problem hiding this comment.
You handlers have a return signature of void, but the actual handler if often async. If svelte-check does catch that, it could be a problem.
Ideally, you would need to make the handler like this, and then you'd be forced to await it.
export let onNewAlbum: (search: string) => Promise<unknown>;There was a problem hiding this comment.
Yeah, I'm trying to figure out why svelte-check/eslint aren't catching it. That's why I sent the eslint PR (#7382), which I expected would catch this. Unfortunately, it's still not. Perhaps it's an issue with eslint-plugin-svelte since it's in the Svelte markup portion of the template. I'm leaving this broken and in draft mode until I track it down as I'm more concerned with having an automated check to avoid the error than fixing this particular instance of it. Next step on that journey is here: #7471
There was a problem hiding this comment.
I filed an issue in the eslint-plugin-svelte repo: sveltejs/eslint-plugin-svelte#691
I'm going to close this PR until that's implemented as it's too hard to keep everything straight without the linter's help
benmccann
force-pushed
the
migrate-more
branch
from
e3c9728
to
91361c3
Compare
Not ready yet... I think it might not be making some things async that should be. I need to check and see why
svelte-checkisn't catching these