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

System.Datetime.TryParse too forgiving in Javascript #3858

Open
HLWeil opened this issue Jun 19, 2024 · 3 comments
Open

System.Datetime.TryParse too forgiving in Javascript #3858

HLWeil opened this issue Jun 19, 2024 · 3 comments

Comments

@HLWeil
Copy link

HLWeil commented Jun 19, 2024

Description

Please provide a succinct description of your issue.

Repro code

https://fable.io/repl/#?code=DYUwLgBARgNAbhAvBAygTwM5hAWwHQAiAhtgCoCWOIepATmgApG0YgAUARAIIBCAwhABsHAJQBYAFCSADrXIA7MADN5EDgGMi8niCYsQAEwBcEAKSwIcIsACuIRKYDyHaJaA&html=Q&css=Q

let b,v = System.DateTime.TryParse("ABC 6")

printfn "canBeParsed: %b, value=%O" b v

Expected and actual results

"ABC 6" should not be parsed as a date. In DotNet and Python, as expected, the bool returned is false. In Javascript though, the bool is true and the number after the "ABC" is parsed as the month.

Related information

  • Fable version: 4.19.3
  • Operating system: Windows 11
@HLWeil
Copy link
Author

HLWeil commented Jun 19, 2024

Would be happy to help out on this one myself. Seems like a good small starting issue.

@MangelMaxime
Copy link
Member

Hello,

The place to investigate this is issue should be:

export function tryParse(v: string, defValue: FSharpRef<IDateTime>): boolean {
try {
defValue.contents = parse(v);
return true;
} catch (_err) {
return false;
}
}

More specifically, I believe this problem is going to be in the parseRaw function:

export function parseRaw(input: string): [Date, Offset] {
function fail() {
throw new Error(`The string is not a valid Date: ${input}`);
}
if (input == null || input.trim() === "") {
fail();
}
// ISO dates without TZ are parsed as UTC. Adding time without TZ keeps them local.
if (input.length === 10 && input[4] === "-" && input[7] === "-") {
input += "T00:00:00";
}
let date = new Date(input);
let offset: Offset = null;
if (isNaN(date.getTime())) {
// Try to check strings JS Date cannot parse (see #1045, #1422)
// tslint:disable-next-line:max-line-length
const m = /^\s*(\d+[^\w\s:]\d+[^\w\s:]\d+)?\s*(\d+:\d+(?::\d+(?:\.\d+)?)?)?\s*([AaPp][Mm])?\s*(Z|[+-]([01]?\d):?([0-5]?\d)?)?\s*$/.exec(input);
if (m != null) {
let baseDate: Date;
let timeInSeconds = 0;
if (m[2] != null) {
const timeParts = m[2].split(":");
const hourPart = parseInt(timeParts[0], 10);
timeInSeconds =
hourPart * 3600 +
parseInt(timeParts[1] || "0", 10) * 60 +
parseFloat(timeParts[2] || "0");
if (m[3] != null && m[3].toUpperCase() === "PM" && hourPart < 12) {
timeInSeconds += 720;
}
}
if (m[4] != null) { // There's an offset, parse as UTC
if (m[1] != null) {
baseDate = new Date(m[1] + " UTC");
} else {
const d = new Date();
baseDate = new Date(d.getUTCFullYear() + "/" + (d.getUTCMonth() + 1) + "/" + d.getUTCDate());
}
if (m[4] === "Z") {
offset = "Z";
} else {
let offsetInMinutes = parseInt(m[5], 10) * 60 + parseInt(m[6] || "0", 10);
if (m[4][0] === "-") {
offsetInMinutes *= -1;
}
offset = offsetInMinutes;
timeInSeconds -= offsetInMinutes * 60;
}
} else {
if (m[1] != null) {
baseDate = new Date(m[1]);
} else {
const d = new Date();
baseDate = new Date(d.getFullYear() + "/" + (d.getMonth() + 1) + "/" + d.getDate());
}
}
date = new Date(baseDate.getTime() + timeInSeconds * 1000);
// correct for daylight savings time
date = new Date(date.getTime() + (date.getTimezoneOffset() - baseDate.getTimezoneOffset()) * 60000);
} else {
fail();
}
// Check again the date is valid after transformations, see #2229
if (isNaN(date.getTime())) {
fail();
}
}
return [date, offset];
}

The easiest way to work on this issue, is to copy your reproduction code in src/quicktest/QuickTest.fs and then run ./build.sh quicktest javascript.

Once, this is running you can edit the Date.js generated under the fable_modules in the src/quicktest folder. Like that you don't need to always recompile everything. Once this is fixed, you can copy your fixes from that temporary file to src/fable-library-ts/Date.ts.

Tip

If it helps in this F# amplifying session https://amplifyingfsharp.io/sessions/2023/12/15/ at around 27min I am showing how > it can be done for Python target. (similar logic applies for JavaScript).

Add a new test to capture this edges case, you can add it to JavaScript, Rust, Python, etc. so it can be checked for the most number of target possible.

@HLWeil
Copy link
Author

HLWeil commented Jun 27, 2024

Hey @MangelMaxime,

thank you very much for the indepth tips and explanations. I will look into it!

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

2 participants