-
Notifications
You must be signed in to change notification settings - Fork 9
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
Support Svelte 5 Runes mode #51
base: master
Are you sure you want to change the base?
Changes from 7 commits
8e697f2
12696a0
5cfeaef
3ec2f88
bacf24d
055d015
d73004b
e9653bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,67 +1,69 @@ | ||
<script> | ||
// @ts-check | ||
|
||
/** | ||
* Original timestamp | ||
* @type {import("dayjs").ConfigType} | ||
*/ | ||
export let timestamp = new Date().toISOString(); | ||
const { | ||
/** | ||
* Original timestamp | ||
* @type {import("dayjs").ConfigType} | ||
*/ | ||
timestamp = new Date().toISOString(), | ||
|
||
/** | ||
* Timestamp format for display. | ||
* It's also used as a title in the `relative` mode | ||
* @type {import("dayjs").OptionType} | ||
* @example "YYYY-MM-DD" | ||
*/ | ||
export let format = "MMM DD, YYYY"; | ||
/** | ||
* Timestamp format for display. | ||
* It's also used as a title in the `relative` mode | ||
* @type {import("dayjs").OptionType} | ||
* @example "YYYY-MM-DD" | ||
*/ | ||
format = "MMM DD, YYYY", | ||
|
||
/** | ||
* Set to `true` to display the relative time from the provided `timestamp`. | ||
* The value is displayed in a human-readable, relative format (e.g., "4 days ago", "Last week") | ||
* @type {boolean} | ||
*/ | ||
export let relative = false; | ||
/** | ||
* Set to `true` to display the relative time from the provided `timestamp`. | ||
* The value is displayed in a human-readable, relative format (e.g., "4 days ago", "Last week") | ||
* @type {boolean} | ||
*/ | ||
relative = false, | ||
|
||
/** | ||
* Set to `true` to update the relative time at 60 second interval. | ||
* Pass in a number (ms) to specify the interval length | ||
* @type {boolean | number} | ||
*/ | ||
export let live = false; | ||
|
||
/** | ||
* Formatted timestamp. | ||
* Result of invoking `dayjs().format()` | ||
* @type {string} | ||
*/ | ||
export let formatted = ""; | ||
/** | ||
* Set to `true` to update the relative time at 60 second interval. | ||
* Pass in a number (ms) to specify the interval length | ||
* @type {boolean | number} | ||
*/ | ||
live = false, | ||
...rest | ||
} = $props(); | ||
|
||
import { dayjs } from "./dayjs"; | ||
import { onMount } from "svelte"; | ||
|
||
/** @type {undefined | NodeJS.Timeout} */ | ||
let interval = undefined; | ||
|
||
const DEFAULT_INTERVAL = 60 * 1_000; | ||
|
||
onMount(() => { | ||
$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); | ||
}); | ||
Comment on lines
+39
to
51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wait how does this even work. If this is setting formatted, but formatted is derived. Guessing this one takes over the derived instance. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good call. it does work, but i agree, it doesn't makes sense for me to update a derived value. i'm thinking but, in this instance, the derived will only be updated if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Either is fine, I think that if we update it the initial state needs to be set in the effect? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i found that in https://svelte.dev/docs/svelte/$effect#Understanding-dependencies There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. with what i found out above, i'm thinking to either keep it the way it is or change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably using state would be good, whatever keeps it simple and easy to understand There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
$: if (relative && live !== false) { | ||
interval = setInterval( | ||
() => { | ||
formatted = dayjs(timestamp).from(); | ||
}, | ||
Math.abs(typeof live === "number" ? live : DEFAULT_INTERVAL), | ||
); | ||
} | ||
$: formatted = relative | ||
? dayjs(timestamp).from() | ||
: dayjs(timestamp).format(format); | ||
$: title = relative ? dayjs(timestamp).format(format) : undefined; | ||
/** | ||
* Formatted timestamp. | ||
* Result of invoking `dayjs().format()` | ||
* @type {string} | ||
*/ | ||
let formatted = $state( | ||
relative ? dayjs(timestamp).from() : dayjs(timestamp).format(format) | ||
); | ||
|
||
const title = $derived( | ||
relative ? dayjs(timestamp).format(format) : undefined, | ||
); | ||
</script> | ||
|
||
<time {title} {...$$restProps} datetime={timestamp}> | ||
<time {title} {...rest} datetime={timestamp}> | ||
{formatted} | ||
</time> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This
exports
condition is not needed, as it's dynamically generated.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed. i originally added it due to a compiler warning