Support Svelte 5 Runes mode #51

kvangrae · 2024-12-10T00:52:07Z

This pull request provides support for Svelte 5 runes mode. It is not backwards compatible for Svelte 4.

Summary of changes:

Dependencies have been upgraded to support Svelte 5.
Compiler has been set to Runes mode.
Code updated to use runes.

Closes #48

niemyjski · 2024-12-10T01:35:15Z

src/Time.svelte


  import { dayjs } from "./dayjs";
  import { onMount } from "svelte";

  /** @type {undefined | NodeJS.Timeout} */
  let interval = undefined;
+  let liveUpdate = $state(0);

  const DEFAULT_INTERVAL = 60 * 1_000;

  onMount(() => {


Seems like this should be called onDestroy https://svelte.dev/docs/svelte/lifecycle-hooks

Actually would be better to move it into $effect

$effect(() => { // This will be recreated whenever `milliseconds` changes const interval = setInterval(() => { count += 1; }, milliseconds); return () => { // if a callback is provided, it will run // a) immediately before the effect re-runs // b) when the component is destroyed clearInterval(interval); }; }); ```

good catch. that should be easy. i'll have a look tomorrow.

i have updated the code to return the clearInterval in $effect return statement.

niemyjski

Looks good, I think we could clean up the onmount and use the function return of $effect

niemyjski · 2024-12-10T01:37:45Z

cc @metonym

kvangrae · 2024-12-10T01:54:19Z

src/Time.svelte


  import { dayjs } from "./dayjs";
  import { onMount } from "svelte";

  /** @type {undefined | NodeJS.Timeout} */
  let interval = undefined;
+  let liveUpdate = $state(0);

  const DEFAULT_INTERVAL = 60 * 1_000;

  onMount(() => {


good catch. that should be easy. i'll have a look tomorrow.

kvangrae · 2024-12-10T01:55:03Z

src/Time.svelte

+  let formatted = $derived.by(() => {
+    liveUpdate; // no-op. this is a dependency trigger for live updates
+    return relative ? dayjs(timestamp).from() : dayjs(timestamp).format(format);
+  });


I don't think that we need the liveUpdate $state rune b/c if any of the props change, formatted should get updated (i think...). it is something i can test. if not, this would simplify some things.

i have removed liveUpdate as it does not have any adverse impacts on live updates.

niemyjski · 2024-12-11T01:03:10Z

src/Time.svelte

+  $effect(() => {
+    /** @type {undefined | NodeJS.Timeout} */
+    let interval;
+    if (relative && live !== false) {
+      interval = setInterval(
+        () => {
+          formatted = dayjs(timestamp).from();
+        },
+        Math.abs(typeof live === "number" ? live : DEFAULT_INTERVAL),
+      );
+    }
    return () => clearInterval(interval);
  });


Wait how does this even work. If this is setting formatted, but formatted is derived. Guessing this one takes over the derived instance.

good call. it does work, but i agree, it doesn't makes sense for me to update a derived value. i'm thinking formatted should be a $state instead of a $derived. if that makes sense, let me know and i will update.

but, in this instance, the derived will only be updated if relative, live, or timestamp ever change, which, most likely, they won't change.

Either is fine, I think that if we update it the initial state needs to be set in the effect?

i found that in $effect any values that are read asynchronously, for example in a setInterval, will not be tracked.

https://svelte.dev/docs/svelte/$effect#Understanding-dependencies

also, it doesn't look like it is recommended to set the initial state inside of the $effect. my intellij complains with the following:

with what i found out above, i'm thinking to either keep it the way it is or change formatted to use $state instead of $derived. let me know your thoughts.

Probably using state would be good, whatever keeps it simple and easy to understand

niemyjski

Looks good, left one comment.

niemyjski · 2024-12-13T16:08:18Z

@metonym can we get a new release and this merged? Is there anything else that should be updated like Dayjs?

metonym · 2024-12-15T18:45:35Z

Nice work, thanks both.

Will review – I think one thing that'll need updating is the demo, for which it breaks entirely for Svelte 5. It could be a nice opportunity to switch to Vite instead of Rollup.

niemyjski · 2024-12-31T03:20:24Z

@metonym can we get this merged and that gets followed up on?

… a svelte field in their package.json but no exports condition for svelte. svelte-time@0.9.0 Please see https://github.com/sveltejs/vite-plugin-svelte/blob/main/docs/faq.md#missing-exports-condition for details.

kvangrae · 2025-01-10T23:38:20Z

Nice work, thanks both.

Will review – I think one thing that'll need updating is the demo, for which it breaks entirely for Svelte 5. It could be a nice opportunity to switch to Vite instead of Rollup.

I've updated and ran the examples. I could successfully run and verify sveltekit and vite. I ran into issues with the following examples:

rollup - npm run dev I couldn't find what port it was running on to verify. I was expecting it to be running on default port 5000.
webpack - npm run dev complains "Module not found: Error: Package path . is not exported from package". I'm not good with webpack and wasn't sure exactly how to resolve it.

Any help would be appreciated.

metonym · 2025-01-11T00:52:13Z

package.json

+    ".": {
+      "svelte": "./src/index.js"
+    }
+  },


This exports condition is not needed, as it's dynamically generated.

removed. i originally added it due to a compiler warning

Vangraefschepe Kurt Colin kcvangr added 3 commits December 9, 2024 23:38

update deps to svelte 5 and vite 4

8e697f2

migrate to svelte 5 runes mode

12696a0

update CHANGELOG

5cfeaef

kvangrae mentioned this pull request Dec 10, 2024

Cannot use $$restProps in runes mode (Svelte 5) #48

Open

niemyjski reviewed Dec 10, 2024

View reviewed changes

niemyjski approved these changes Dec 10, 2024

View reviewed changes

kvangrae commented Dec 10, 2024

View reviewed changes

Vangraefschepe Kurt Colin kcvangr added 2 commits December 10, 2024 16:16

move clearInterval into the return of $effect

3ec2f88

remove unnecessary liveUpdate state variable

bacf24d

niemyjski reviewed Dec 11, 2024

View reviewed changes

niemyjski approved these changes Dec 11, 2024

View reviewed changes

use $state instead of $derived

055d015

niemyjski approved these changes Dec 13, 2024

View reviewed changes

metonym reviewed Jan 11, 2025

View reviewed changes

remove exports as it's dynamically generated

e9653bb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Svelte 5 Runes mode #51

Support Svelte 5 Runes mode #51

kvangrae commented Dec 10, 2024

niemyjski Dec 10, 2024

niemyjski Dec 10, 2024

kvangrae Dec 10, 2024

kvangrae Dec 10, 2024

niemyjski left a comment

niemyjski commented Dec 10, 2024

kvangrae Dec 10, 2024

kvangrae Dec 10, 2024

kvangrae Dec 10, 2024

niemyjski Dec 11, 2024 •

edited

Loading

kvangrae Dec 11, 2024

niemyjski Dec 11, 2024

kvangrae Dec 11, 2024

kvangrae Dec 11, 2024

kvangrae Dec 11, 2024 •

edited

Loading

niemyjski Dec 12, 2024

kvangrae Dec 13, 2024

niemyjski left a comment

niemyjski commented Dec 13, 2024

metonym commented Dec 15, 2024

niemyjski commented Dec 31, 2024

kvangrae commented Jan 10, 2025

metonym Jan 11, 2025

kvangrae Jan 14, 2025

Support Svelte 5 Runes mode #51

Are you sure you want to change the base?

Support Svelte 5 Runes mode #51

Conversation

kvangrae commented Dec 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niemyjski left a comment

Choose a reason for hiding this comment

niemyjski commented Dec 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niemyjski Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kvangrae Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niemyjski left a comment

Choose a reason for hiding this comment

niemyjski commented Dec 13, 2024

metonym commented Dec 15, 2024

niemyjski commented Dec 31, 2024

kvangrae commented Jan 10, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niemyjski Dec 11, 2024 •

edited

Loading

kvangrae Dec 11, 2024 •

edited

Loading