Skip to content

Commit b82bdcd

Browse files
spahnkeoliverbock
authored andcommitted
Fix conversion of dates from C# to V8
We need to use the date constructor to create a date, otherwise the order of invocation of the set methods influences the result leading to incorrect date values. There is no permutation which is always correct. Example: 1967-04-01 00:00:00 Depending on the timezone the milliseconds from C# could represent 1967-03-31 23:00:00. If we now set the month to April we get the 31st of April which JavaScript interprets as the 1st of May. Even if we don't use the current date as starting point but 1970-01-01 00:00:00 using the set methods can lead to errors. Example: 1972-02-29 00:00:00 Since we start from 1970-01-01 00:00:00 (UTC) which was not a leap year, we need to set the year first. Otherwise, if we now set the month to February and the date to 29 we get the 1st of March. Another case is can occur when changing daylight savings time.
1 parent 262c5ac commit b82bdcd

File tree

2 files changed

+96
-20
lines changed

2 files changed

+96
-20
lines changed

Source/Noesis.Javascript/JavascriptInterop.cpp

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -387,13 +387,6 @@ double GetDateComponent(Isolate* isolate, Handle<Date> date, const char* compone
387387
return componentValue->NumberValue(isolate->GetCurrentContext()).ToChecked();
388388
}
389389

390-
void SetDateComponent(Isolate* isolate, Handle<Date> date, const char* component, double value)
391-
{
392-
auto getComponent = date->Get(isolate->GetCurrentContext(), String::NewFromUtf8(isolate, component)).ToLocalChecked().As<Function>();
393-
Handle<Value> parameters[] = { JavascriptInterop::ConvertToV8(value) };
394-
getComponent->Call(date, 1, parameters);
395-
}
396-
397390
System::DateTime^ JavascriptInterop::ConvertDateFromV8(Handle<Date> date)
398391
{
399392
auto isolate = JavascriptContext::GetCurrentIsolate();
@@ -412,18 +405,22 @@ Handle<Date> JavascriptInterop::ConvertDateTimeToV8(System::DateTime^ dateTime)
412405
auto isolate = JavascriptContext::GetCurrentIsolate();
413406
EscapableHandleScope handleScope(isolate);
414407

415-
auto date = v8::Date::New(isolate->GetCurrentContext(), SystemInterop::ConvertFromSystemDateTime(dateTime)).ToLocalChecked().As<Date>();
416408
if (dateTime->Kind == System::DateTimeKind::Utc)
417-
return handleScope.Escape(date);
418-
419-
SetDateComponent(isolate, date, "setFullYear", dateTime->Year);
420-
SetDateComponent(isolate, date, "setMonth", dateTime->Month - 1);
421-
SetDateComponent(isolate, date, "setDate", dateTime->Day);
422-
SetDateComponent(isolate, date, "setHours", dateTime->Hour);
423-
SetDateComponent(isolate, date, "setMinutes", dateTime->Minute);
424-
SetDateComponent(isolate, date, "setSeconds", dateTime->Second);
425-
SetDateComponent(isolate, date, "setMilliseconds", dateTime->Millisecond);
426-
return handleScope.Escape(date);
409+
return handleScope.Escape(v8::Date::New(isolate->GetCurrentContext(), SystemInterop::ConvertFromSystemDateTime(dateTime)).ToLocalChecked().As<Date>());
410+
411+
auto year = JavascriptInterop::ConvertToV8(dateTime->Year);
412+
auto month = JavascriptInterop::ConvertToV8(dateTime->Month - 1);
413+
auto day = JavascriptInterop::ConvertToV8(dateTime->Day);
414+
auto hour = JavascriptInterop::ConvertToV8(dateTime->Hour);
415+
auto minute = JavascriptInterop::ConvertToV8(dateTime->Minute);
416+
auto second = JavascriptInterop::ConvertToV8(dateTime->Second);
417+
auto millisecond = JavascriptInterop::ConvertToV8(dateTime->Millisecond);
418+
419+
auto globalObj = isolate->GetCurrentContext()->Global();
420+
auto dateConstructor = Local<Function>::Cast(globalObj->Get(String::NewFromUtf8(isolate, "Date")));
421+
Handle<Value> parameters[] = { year, month, day, hour, minute, second, millisecond };
422+
423+
return handleScope.Escape(dateConstructor->NewInstance(isolate->GetCurrentContext(), 7, parameters).ToLocalChecked().As<Date>());
427424
}
428425

429426
////////////////////////////////////////////////////////////////////////////////////////////////////

Tests/Noesis.Javascript.Tests/DateTest.cs

Lines changed: 81 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
using System;
22
using Microsoft.VisualStudio.TestTools.UnitTesting;
33
using FluentAssertions;
4+
using System.Collections.Generic;
5+
using System.Linq;
46

57
namespace Noesis.Javascript.Tests
68
{
@@ -115,8 +117,7 @@ public void SetDateTimeLocal_DateWhereTimezoneDatabaseIsImportant()
115117
_context.Run("val.getSeconds()").Should().BeOfType<int>().Which.Should().Be(0);
116118
}
117119

118-
[TestMethod]
119-
[Ignore]
120+
//[TestMethod]
120121
public void SetAndReadDateTimeUtc_DateWhereTimezoneDatabaseIsImportant()
121122
{
122123
var dateTime = new DateTime(1978, 6, 15, 0, 0, 0, DateTimeKind.Utc);
@@ -126,6 +127,26 @@ public void SetAndReadDateTimeUtc_DateWhereTimezoneDatabaseIsImportant()
126127
dateFromV8.ToUniversalTime().Should().Be(dateTime); // this cannot work without an external dependency like NodaTime
127128
}
128129

130+
//[TestMethod]
131+
public void SetAndReadDateTimeUtc_EdgeCaseOfAMonthWith30Days()
132+
{
133+
var dateTime = new DateTime(1967, 4, 1, 0, 0, 0, DateTimeKind.Utc);
134+
_context.SetParameter("val", dateTime);
135+
136+
var dateFromV8 = (DateTime) _context.Run("val");
137+
dateFromV8.ToUniversalTime().Should().Be(dateTime); // this cannot work without an external dependency like NodaTime
138+
}
139+
140+
//[TestMethod]
141+
public void SetAndReadDateTimeUtc_LeapYear()
142+
{
143+
var dateTime = new DateTime(1972, 2, 29, 0, 0, 0, DateTimeKind.Utc);
144+
_context.SetParameter("val", dateTime);
145+
146+
var dateFromV8 = (DateTime) _context.Run("val");
147+
dateFromV8.ToUniversalTime().Should().Be(dateTime); // this cannot work without an external dependency like NodaTime
148+
}
149+
129150
[TestMethod]
130151
public void SetAndReadDateTimeLocal_DateWhereTimezoneDatabaseIsImportant()
131152
{
@@ -135,6 +156,24 @@ public void SetAndReadDateTimeLocal_DateWhereTimezoneDatabaseIsImportant()
135156
_context.Run("val").Should().Be(dateTime);
136157
}
137158

159+
[TestMethod]
160+
public void SetAndReadDateTimeLocal_EdgeCaseOfAMonthWith30Days()
161+
{
162+
var dateTime = new DateTime(1967, 4, 1, 0, 0, 0, DateTimeKind.Local);
163+
_context.SetParameter("val", dateTime);
164+
165+
_context.Run("val").Should().Be(dateTime);
166+
}
167+
168+
[TestMethod]
169+
public void SetAndReadDateTimeLocal_LeapYear()
170+
{
171+
var dateTime = new DateTime(1972, 2, 29, 0, 0, 0, DateTimeKind.Local);
172+
_context.SetParameter("val", dateTime);
173+
174+
_context.Run("val").Should().Be(dateTime);
175+
}
176+
138177
[TestMethod]
139178
public void SetAndReadDateTimeUnspecified_DateWhereTimezoneDatabaseIsImportant()
140179
{
@@ -144,11 +183,51 @@ public void SetAndReadDateTimeUnspecified_DateWhereTimezoneDatabaseIsImportant()
144183
_context.Run("val").Should().Be(dateTime);
145184
}
146185

186+
[TestMethod]
187+
public void SetAndReadDateTimeUnspecified_EdgeCaseOfAMonthWith30Days()
188+
{
189+
var dateTime = new DateTime(1967, 4, 1);
190+
_context.SetParameter("val", dateTime);
191+
192+
_context.Run("val").Should().Be(dateTime);
193+
}
194+
195+
[TestMethod]
196+
public void SetAndReadDateTimeUnspecified_LeapYear()
197+
{
198+
var dateTime = new DateTime(1972, 2, 29);
199+
_context.SetParameter("val", dateTime);
200+
201+
_context.Run("val").Should().Be(dateTime);
202+
}
203+
147204
[TestMethod]
148205
public void CreateFixedDateInJavaScript_DateWhereTimezoneDatabaseIsImportant()
149206
{
150207
DateTime dateAsReportedByV8 = (DateTime) _context.Run("new Date(1978, 5, 15)");
151208
dateAsReportedByV8.Should().Be(new DateTime(1978, 6, 15));
152209
}
210+
211+
[TestMethod]
212+
public void SetAndReadDateTimeLocal_DateRange()
213+
{
214+
var startDate = new DateTime(1900, 1, 1, 0, 0, 0, DateTimeKind.Local);
215+
var endDate = new DateTime(2100, 1, 1, 0, 0, 0, DateTimeKind.Local);
216+
var totalDays = (endDate - startDate).TotalDays;
217+
218+
var errorDates = new List<string>();
219+
220+
for (var i = 0; i < totalDays; i++)
221+
{
222+
var date = startDate.AddDays(i);
223+
_context.SetParameter("val", date);
224+
var result = (DateTime) _context.Run("val");
225+
if (result != date)
226+
errorDates.Add($"Expected {date}, but got {result}");
227+
}
228+
229+
if (errorDates.Any())
230+
Assert.Fail(string.Join("\n", errorDates));
231+
}
153232
}
154233
}

0 commit comments

Comments
 (0)