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

[feat] Props data checker/converter #13124

Open
adiguba opened this issue Sep 4, 2024 · 7 comments
Open

[feat] Props data checker/converter #13124

adiguba opened this issue Sep 4, 2024 · 7 comments

Comments

@adiguba
Copy link
Contributor

adiguba commented Sep 4, 2024

Describe the problem

Sometimes it can be useful to validate or convert the value of a prop before using it.

Some examples :

  • convert a string to another type (eg: number).
  • replace incorrect values ​​with a default value.
  • respect other criteria such as a min/max.
  • I want to use a different type internally an externally (ex date string format and Date object)

And of course I would like to keep this responsive and recheck the prop every time it is changed from outside


Currently I see two way to do this.


  1. If the prop is read-only, I can use a $derived() state :
<script>
	let {
		// the prop "value" is renamed "propValue" inside the component
		value : propValue = 0
	} = $props();

	// the "propValue" is converted to Number, into a "value" state
	const value = $derived(Number(propValue ));
</script>

This is fine, even if it involves duplicating the props.


  1. If the prop is modified inside the component, I have to use a $effect.pre() to validate the value on any update :
	let {
		value = 0
	} = $props();


	$effect.pre( () => {
		const value_as_number = Number(value);
		if (value_as_number !== value) {
			value = value_as_number;
		}
	});
	
</script>

This solution is more verbose, and the effect is re-executed even when I change the value inside my component (useless).

Describe the proposed solution

I think it would be nice to have a way to replace the prop's default value with a custom function.
Something like this (using a $pipe rune for the exemple - but the name should be reviewed) :

<script>
	let {
		// when the prop "value" is updated by the caller,
		// it will be converted to number
		value = $pipe( Number )
	} = $props();
</script>

The $pipe() rune requires a function as a parameter, which will be used to convert the value of the prop.
There is no default value, it is up to the function to handle the undefined case...

Of course, we should have the same with $bindable(), something like this :

<script>
	let {
		// when the prop "value" is updated by the caller,
		// it will be converted to number
		// (and binded to the caller with his new value)
		value = $bindable.pipe( Number )
	} = $props();
</script>

$bindable.pipe() should accept a second function, allowing the reverse conversion to be done when the value is passed back to the caller

<script>
	let {
		// when the prop "date" is updated by the caller, 
		// it will be converted to a Date object
		// And it will be binded to the caller as an ISO string.
		date = $bindable.pipe(
			// Date is converted from string to Date
			(d)=>new Date(Date.parse(d)),
			// Date is binded to caller as string
			(d)=>d.toISOString()
		)
	} = $props();
</script>

Caveat :

I don't known how to handle this with TypeScript, as the prop should have two distinct type.
From the caller perspective it should be a string, but in the component it should be a Date

It would be nice to have a simple way to declare this, like something like that :

type Props = {
  date: PROP<string, Date>
};

But I don't know if it's easily achievable...

Importance

nice to have

@dummdidumm
Copy link
Member

You can use an object with a getter/setter to achieve this:

<script>
  let { value = $bindable() } = $props();

  let proxied = {
    get value() { return new Date(Date.parse(d)) },
    set value(v) { value = v.toISOString() }
  }
</script>

<... bind:value={proxied.value} />

@adiguba
Copy link
Contributor Author

adiguba commented Sep 4, 2024

This solution is great from the caller perspective (typically to pass to a bind:value).

But if I use the {proxied.value} multiple time on my template, the conversion will be repeated every time.
And it seems this don't work with reactive object like SvelteDate.

I will try to use this with a $derived...

@adiguba
Copy link
Contributor Author

adiguba commented Sep 5, 2024

I didn't succeed with getter/setter with a reactive value (it update the value too frequently).

So I use two $effect.pre() :

	let {
		date : dateAsString = $bindable()
	} = $props();

	let date = $state();
	let date_value;
	$effect.pre( () => {
		// when dateAsString change, update the date :
		if (date_value !== dateAsString) {
			date = new SvelteDate(dateAsString);
		}
	});
	$effect.pre(() => {
		// when date change, update the dateAsString
		date_value = date.toISOString().substring(0,10);
		if (date_value !== untrack(()=>dateAsString)) {
			dateAsString = date_value;
		}
	});

See here : REPL

It's work, but it's a bit verbose...

I can make it an external function, but then I have to pass the getter/setter for the state and the prop :

	let date = $state();
	pipe(
		// get_state
		() => date,
		// set_state
		(new_state) => date = new_state,
		// get_prop
		() => dateAsString,
		// set_prop
		(new_prop) => dateAsString = new_prop,
		// prop_to_state
		(prop) => new SvelteDate(prop),
		// state_to_prop
		(state) => state.toISOString().substring(0,10)
	);

Link : REPL

@trueadm
Copy link
Contributor

trueadm commented Sep 5, 2024

Rather than passing around date as a prop, you can instead pass around a class instance where you can control the things you care about. You shouldn't be using effects to keep things in sync because they're updated on a microtask, meaning you'll have a glitchy experience. So rather than "converting" props to a different type, you just have a contract where people provide the correct type to begin with and a good way of enforcing that safely is with a class.

Alternatively, if you're transforming data, then use $derived – if you're wanting to mutate that value back out then you should expose a function between components to do that specific behaviour, rather than with effects. If you can provide me with a real-world REPL, I can update it to show you what I mean.

@Thiagolino8
Copy link

This solution is great from the caller perspective (typically to pass to a bind:value).

But if I use the {proxied.value} multiple time on my template, the conversion will be repeated every time

Then add one extra line

let { value = $bindable() } = $props();
const getter = $derived(new Date(Date.parse(d)));

let proxied = {
  get value() { return getter},
  set value(v) { value = v.toISOString() }
}

@adiguba
Copy link
Contributor Author

adiguba commented Sep 10, 2024

@trueadm : yes but I don't want to pass a class instance. I just want a simple string.

A real-life example would be a DateInput component.

<script>
	let date = $state("2024-09-10");
</script>

<DateInput bind:value={date} />

The component should be called with a string representing the date, but inside my component I want to use a SvelteDate (or another date-library) to handle correctly the calendar.

I want to use this component like an <input type="date">, with all the conversion stuff would be done within the component (not by the caller).

Exemple here : REPL

Currently, I have to use $effect for that...

@trueadm
Copy link
Contributor

trueadm commented Sep 10, 2024

You don't need effects, you can just use objects like mentioned above – or create your own proxied object. However, given you want a SvelteDate you can also model this on your own object instead – which is far better than using strings, because strings are only simple if you're using them in a simple abstraction – which you're not doing in that REPL.

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

No branches or pull requests

4 participants