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

[Svelte 5] Deprecations/warnings #249

Open
NickMous opened this issue Sep 23, 2024 · 5 comments
Open

[Svelte 5] Deprecations/warnings #249

NickMous opened this issue Sep 23, 2024 · 5 comments
Labels
enhancement New feature or request
Milestone

Comments

@NickMous
Copy link

Hi there!
While installing and using svelecte, I noticed there are some warnings while compiling this package, which unfortunately break our end2end tests.

Warnings of Svelecte that are present:

 warn  in ./node_modules/svelecte/dist/Svelecte.svelte                                                                                                                          10:45:18 AM

Module Warning (from ./node_modules/svelte-loader/index.js):
a11y_consider_explicit_label: Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute
node_modules/svelecte/dist/Svelecte.svelte:1764:4
1762:     </div>
1763:     {#if multiple}
1764:     <button class="sv-item--btn" tabindex="-1" type="button"
                   ^
1765:       data-action="deselect"
1766:       use:bindItemAction={opt}

 warn  in ./node_modules/svelecte/dist/Svelecte.svelte                                                                                                                          10:45:18 AM

Module Warning (from ./node_modules/svelte-loader/index.js):
reactive_declaration_non_reactive_property: Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update
node_modules/svelecte/dist/Svelecte.svelte:349:33
347:   $: itemRenderer = typeof renderer === 'function'
348:     ? renderer
349:     : (renderer !== 'default' && stringFormatters[renderer]
                                                                ^
350:       ? stringFormatters[renderer]
351:       : stringFormatters.default.bind({ label: currentLabelField})

 warn  in ./node_modules/svelecte/dist/Svelecte.svelte                                                                                                                          10:45:18 AM

Module Warning (from ./node_modules/svelte-loader/index.js):
reactive_declaration_non_reactive_property: Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update
node_modules/svelecte/dist/Svelecte.svelte:350:8
348:     ? renderer
349:     : (renderer !== 'default' && stringFormatters[renderer]
350:       ? stringFormatters[renderer]
                                       ^
351:       : stringFormatters.default.bind({ label: currentLabelField})
352:     );

 warn  in ./node_modules/svelecte/dist/Svelecte.svelte                                                                                                                          10:45:18 AM

Module Warning (from ./node_modules/svelte-loader/index.js):
script_context_deprecated: `context="module"` is deprecated, use the `module` attribute instead
node_modules/svelecte/dist/Svelecte.svelte:1:8
1: <script context="module">
                           ^
2:   import defaults from './settings.js';
3:

To take some work off your shoulders, so you can focus on improving svelecte overall, I'm going to try and fix those warnings for you if you want.
Don't hesitate to share thoughts!

@NickMous
Copy link
Author

NickMous commented Sep 23, 2024

Made the first steps in a separate branch. Unfortunately I'm unable to push the branch to this repo.
@mskocik What's your (ideal) workflow for contributions? Or are you able to give me permissions to push branches to this repo?

Currently stuck on another dependency that's also a bit behind, I'll go over there and try to fix the problem, so svelecte can go ahead with the most recent svelte 5 version!

[plugin:vite-plugin-svelte] node_modules/.pnpm/[email protected]/node_modules/svelte-tiny-virtual-list/src/VirtualList.svelte:35:1 Cannot use `export let` in runes mode — use `$props()` instead
node_modules/.pnpm/[email protected]/node_modules/svelte-tiny-virtual-list/src/VirtualList.svelte:35:1
33 |    } from './constants';
 34 |  
 35 |    export let height;
                           ^
 36 |    export let width = '100%';
 37 |

Small update already:
Svelte 5 support for svelte-tiny-virtual-list is coming, we'll have to wait for that I think before enabling rune mode!

@mskocik
Copy link
Owner

mskocik commented Sep 23, 2024

What's your (ideal) workflow for contributions?

Normal workflow is you fork the repo, submit changes to your branch (master or another), then create pull request.

I am not sure, it's required for tiny-virtual-list to be also on svelte 5.

Don't hesitate to share thoughts!

I tried to update to the latest version of svelte 5 during the weekend, but tests were failing and I really wasn't in the mood of finding why.

If you e2e tests are failing maybe you could relax some rules to allow warnings?

@mskocik mskocik added this to the v5.0 milestone Sep 23, 2024
@mskocik
Copy link
Owner

mskocik commented Sep 23, 2024

Also I hope we are talking about svelecte@next aka master branch, not svelecte 4 :)

@NickMous
Copy link
Author

Alright! Then I'm gonna make a fork when I notice svelte-tiny-virtual-list is compatible with svelte 5!
I now branched off of the tag v5.0.0-next.14 :)

We work with webpack, and things seem to be a bit gimmicky with warnings and errors. It compiles correctly, shows everything when you look at the served page, but when running tests it doesn't wanna show it anymore with warnings for some reason. Currently looking into it why, but it's really irritating that some things just don't work with testing, while it works when you're looking at the served page :(

Copy link

This issue is stale because it has been open 21 days with no activity. Remove stale label or comment or this will be closed in 3 days.

@github-actions github-actions bot added the Stale label Oct 15, 2024
@mskocik mskocik added enhancement New feature or request and removed Stale labels Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants