ciscoheat / sveltekit-superforms

Making SvelteKit forms a pleasure to use!
https://superforms.rocks
MIT License
2.13k stars 63 forks source link

Bug with local timezone, dateProxy and date input field #84

Closed thelinuxlich closed 1 year ago

thelinuxlich commented 1 year ago

Description My timezone is -03:00, right now if I add a dateProxy to a date input, whenever I select a field, it will select the day before.

If applicable, a MRE Use this stackblitz template to create a minimal reproducible example that you can link to here:

https://stackblitz.com/edit/sveltekit-superforms-bug-reporting-ek13fm?file=src%2Froutes%2F%2Bpage.server.ts,src%2Froutes%2F%2Bpage.svelte

thelinuxlich commented 1 year ago

Another thing I noticed: when using z.date({coerce: true}) fields, on page.server.ts the date is parsed correctly, but on the client-side it applies the local timezone and messes it up

thelinuxlich commented 1 year ago

Made some changes to showcase this behavior, notice the server log with the birthdate value and then the same value on the browser: https://stackblitz.com/edit/sveltekit-superforms-bug-reporting-ek13fm?file=src/routes/+page.server.ts

thelinuxlich commented 1 year ago

Found the fix: https://stackblitz.com/edit/sveltekit-superforms-bug-reporting-ek13fm?file=src/routes/+page.server.ts

It's two things:

ciscoheat commented 1 year ago

Thank you very much for your investigation, I'll do some experimentation, trying to make this consistent!

ciscoheat commented 1 year ago

@thelinuxlich Could you clarify what was the problem you had? I've tested with a fork of your project: https://stackblitz.com/edit/sveltekit-superforms-bug-reporting-91avkz

I've commented out the date transform just for testing, then changed dates and submitted several times, but I cannot see that there is a discrepancy between any dates. They are the same both on server and on client.

thelinuxlich commented 1 year ago

You need to change the locale/region of your system. For example, I'm in Sao Paulo - Brazil, which is timezone GMT -03:00. On the server the timezone doesn't get applied, it's UTC. On the browser it does, so a UTC date becomes 1 day before because of the -03:00.

ciscoheat commented 1 year ago

I'm at +02:00, but you mean it happens when you are in a negative timezone?

thelinuxlich commented 1 year ago

yeah, because at +02:00 a UTC date like 1984-09-02T00:00:00.000Z won't change the day, right?

ciscoheat commented 1 year ago

Yes, that will mess things up because there is only a "date-local" format. I was considering adding a utc format as well, but was hoping it wouldn't be needed. Here's to hoping... :)

When adding that format, it seems to work, though I'm not sure if "date-utc" is semantically correct. It should really mean "use the date part, no matter what timezone", since the date input is a string value, and converting it to date may actually be problematic?

thelinuxlich commented 1 year ago

I think date-utc is correct, while you are at it, may add datetime-utc for datetime fields too. I don't think the conversion would be problematic, the source code for the proxies has a toValue function already doing new Date(val)

ciscoheat commented 1 year ago

Yes, already added that. :) It should be fine, so I'll add this for the update tomorrow.

thelinuxlich commented 1 year ago

I think you'll need to change this line to convert to UTC too when it's "date-utc": https://github.com/ciscoheat/sveltekit-superforms/blob/main/src/lib/client/proxies.ts#L100

Otherwise the z.date() field receives new Date("YYYY-mm-dd") and goes to the server with the local timezone

ciscoheat commented 1 year ago

If you convert it to UTC at that point, which will happen when you change date in the input field for example, it will for negative timezones go backwards, making the input show one day earlier again.

I think the problem is that the date input doesn't care about time more than setting it to midnight at the current timezone. That's why I think describing the format as "date-utc" could be a bit misleading. Maybe a final option is to add just "date", that strips the rest of the ISO string?

thelinuxlich commented 1 year ago

Which is why I added the transform()

ciscoheat commented 1 year ago

I have pushed the fixes now to the repo, not released a npm version yet. As it is now, if you instantiate the date as:

const date = new Date('1984-09-02');

And use the date (or date-utc) format, it works without any transform:

let proxy = dateProxy(form, 'proxy', { format: 'date' });
2023-04-03 18_43_54-localhost_5173_dates

Does this sound good, or can you find some case where it will be problem?

thelinuxlich commented 1 year ago

if the date remains 1984-09-02 after the date proxy sends the update to the $form field and back to the backend, then yeah it's solved

thelinuxlich commented 1 year ago

I tested your changes locally and it seems the date input is behaving normally, date comes back to the backend untouched, nice!

xmlking commented 1 year ago

I had a use case where I have to deal with datetime in UTC formate in the backed (GraphQL/Postgres) but converted to local datetime in the browser.

I had to implemente DateInput component which is used here

dateProxy looks like correct way to implement this. Looking forward for this solution…

ciscoheat commented 1 year ago

Excellent! I've just released 0.6.7, this was a good improvement. Thank you for all your help.

thelinuxlich commented 1 year ago

Just confirmed the bug has been fixed!