search

fable-compiler / Fable

F# to JavaScript, TypeScript, Python, Rust and Dart Compiler
http://fable.io/
MIT License
2.89k stars 296 forks source link

System.Datetime.TryParse too forgiving in Javascript #3858

Open HLWeil opened 3 months ago

HLWeil commented 3 months ago

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

HLWeil commented 3 months ago

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

MangelMaxime commented 3 months ago

Hello,

The place to investigate this is issue should be:

https://github.com/fable-compiler/Fable/blob/8f7ab698eac0d02df1bed21045d8081492a3c26b/src/fable-library-ts/Date.ts#L594-L601

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

https://github.com/fable-compiler/Fable/blob/8f7ab698eac0d02df1bed21045d8081492a3c26b/src/fable-library-ts/Date.ts#L509-L582

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 commented 2 months ago

Hey @MangelMaxime,

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