concerto icon indicating copy to clipboard operation
concerto copied to clipboard

Make serializer option `strictQualifiedDateTimes` default behaviour

Open mttrbrts opened this issue 1 year ago • 4 comments

In concerto-core/lib/serializer/JSONPopulator.js, the constructor option strictQualifiedDateTimes should be the default behaviour to address the issues discussed in #552 .

This is a breaking change, and so needs to be performed against the v4.0 branch.

mttrbrts avatar May 01 '24 08:05 mttrbrts

@mttrbrts ,can I be assigned to it? I'd love to work on it!

darksapien23151 avatar Dec 24 '24 02:12 darksapien23151

@darksapien23151 do you have root cause analysis or initial approach to tackle this?

sanketshevkar avatar Jan 02 '25 05:01 sanketshevkar

Root Cause Analysis & Fix for Concerto Date-Time Issue

🚨 The Issue

When using Concerto, date-time values without an explicit time zone behave unexpectedly. Instead of assuming the local system time, Concerto defaults to UTC (offset 0). This leads to incorrect date shifts based on the user's time zone.

🔍 Example Problem

If you're in UTC-8 and enter:

{"date": "2020-01-01"}

Concerto assumes it’s UTC, so when converted to local time, it shifts backward:

Expected:  2020-01-01T00:00:00-08:00
Actual:    2019-12-31T16:00:00-08:00

Oops! Your date just traveled back in time ⏳😅


🔑 Root Cause

  1. Concerto defaults to UTC for unqualified date-time values, instead of using the system's local time zone.
  2. ISO 8601 states that if no time zone is given, the system should assume local time. But Concerto doesn’t do this.
  3. This affects JSON parsing, causing unexpected shifts when converting dates.

🛠️ How to Fix It

✅ Step 1: Change Default Behavior

  • Modify JSONPopulator.js so strictQualifiedDateTimes defaults to true.
  • This prevents incorrect auto-conversions.

✅ Step 2: Detect Local Time Zone

  • Use JavaScript’s built-in tools to detect the local time zone:
    Intl.DateTimeFormat().resolvedOptions().timeZone;
    
  • Alternatively, use dayjs with timezone support:
    dayjs.tz.guess();
    

✅ Step 3: Allow Users to Set Their Own Offset

  • Introduce a --utcOffset CLI option to let users override the default behavior.

✅ Step 4: Update Concerto CLI Validation

  • Make sure unqualified date-time values always default to local time unless overridden.

✅ Step 5: Ensure Backward Compatibility

  • This change is breaking, so it should be part of Concerto v4.0.
  • Provide clear migration steps for users who rely on the old UTC-based behavior.

🎯 Summary

🚫 Before: Concerto assumes all dates are UTC, causing unexpected shifts. ✅ After Fix: Concerto correctly detects the local time zone, avoiding wrong date conversions.

c-surendra-kumar avatar Mar 15 '25 09:03 c-surendra-kumar

Hi @sanketshevkar, I’m interested in solving this issue. Can you assign it to me?

From the issue, I understand that in concerto-core/lib/serializer/JSONPopulator.js, unqualified date-times are assumed to be in UTC by default. This can cause date shifts for users in different time zones.

My Approach:

  1. Update JSONPopulator.js to set strictQualifiedDateTimes to true by default.
  2. Make unqualified date times use the user’s local time.
  3. Update tests to check the new behavior in different time zones.

Since this change breaks the current behavior, I think the documentation should be updated as well.

Let me know what you think. Would love to start on this soon.

fuyalasmit avatar Mar 17 '25 16:03 fuyalasmit

Hi @mttrbrts , it seems the issue is solved and PR was merged and closed. Is there anything else that's required to be solved? I would be happy to contribute.

rnema19 avatar Oct 19 '25 08:10 rnema19

Thanks for the offer @rnema19, but this issue has already been fixed. I'll close the issue

mttrbrts avatar Oct 20 '25 20:10 mttrbrts