[JS/TS] add tests for nullness supports

[MangelMaxime]

Fix #3887
Fix #3991

@kerams

It seems like Fable already supports "nullness" and its new API thanks to @ncave adding the new NonNull, Null, APIs.

I tried to create some tests suites to check if "nullness" is working but I think that right now this is more testing if T | null and related APIs works with Fable.

I am not sure if we want to enable on the test project <Nullable>enable</Nullable>, this generates several new warnings and I am not sure if this adds value for this specific project.

Example of places with warnings:

If we thinks that enabling nullable make sense, I can give a try to fix them.

I am not familiar with nullness API, @kerams do you see a common case that is not covered and you would like to be covered? I tried to look at F# repository for tests regarding but it seems like most of them are related to testing compiler warning.

[kerams]

Off the top of my head, adding nullness to input parameters of some Fable.Core functions. More often than not it will be changing obj -> objnull.

• JsInterop.isNullOrUndefined
• JsInterop.emitJsExpr (you can pass null or () if you don't refer to the parameters)
• JsInterop.emitJsStatement
• JsInterop.jsTypeOf
• JS.Constructors.Array.isArray
• etc.

There's a LOT more, but the functions can be easily adjusted on demand later. The only downside is the signature in Fable.Core has to be hidden behind #if FABLE_COMPILER_5 || NUGET_BUILD.

[kerams]

I am not sure if we want to enable on the test project enable, this generates several new warnings and I am not sure if this adds value for this specific project.

The problem is, as far as I can see F# does not respect #nullable enable, so without <Nullable>enable</Nullable> you are not checking whether Fable actually shows warnings and enforces them in combination with TreatWarningsAsErrors. If you just test it manually for now, I'll believe you:)

[MangelMaxime]

Off the top of my head, adding nullness to input parameters of some Fable.Core functions. More often than not it will be changing obj -> objnull.

* `JsInterop.isNullOrUndefined`

* `JsInterop.emitJsExpr` (you can pass `null` or `()` if you don't refer to the parameters)

* `JsInterop.emitJsStatement`

* `JsInterop.jsTypeOf`

* `JS.Constructors.Array.isArray`

* etc.

There's a LOT more, but the functions can be easily adjusted on demand later. The only downside is the signature in Fable.Core has to be hidden behind #if FABLE_COMPILER_5 || NUGET_BUILD.

Here, I think you are not speaking about nullness support directly but more shaping Fable.Core API to takes advantages of nullness.

Fable.Core is a pure binding, meaning that it is served as a .dll only not fable folder with the sources files. This means that the compiler directives will have no effect.

We will need to check if nullness information are backward compatible in general. But if we move to use | null instead of Option in the API design this will probably be a major bump because of breaking changes.

The problem is, as far as I can see F# does not respect #nullable enable, so without <Nullable>enable</Nullable> you are not checking whether Fable actually shows warnings and enforces them in combination with TreatWarningsAsErrors. If you just test it manually for now, I'll believe you:)

You are right neither the warning or error is reported by Fable right now. I will take a look why we don't get the diagnostics (perhaps we are not ignoring it or not passing the configuration to FCS).

[kerams]

We will need to check if nullness information are backward compatible in general

It is, so if Fable.Core is not distributed in source form, we don't need directives.

[MangelMaxime]

Nullness warnings are now reported.

Supporting TreatWarningsAsErrors is a whole other task.

• TreatWarningsAsErrors does not treat warnings as errors #2521
• Fable 3 handles <TreatWarningsAsErrors> in referenced projects differently than Fable 2 #2288

This is because Fable recreate a "single project" using the user project and dependencies. So when enabling, TreatWarningsAsErrors the compilation could fails because of a warning coming from a dependencies project.

[MangelMaxime]

It seems like when we enable nullness F# generates errors in certain situation when a constraint don't have the not null constraint.

Below is the output of using Fable with <Nullable> enabled on the main project.

I suspect this is because Fable compile the sources files of the packages where in a normal situation F# just consume the dll and if it don't have the nullable metadata then it is fine with it.

This seems to show that in order to support nullness in Fable ecosystem, we will need to update all the libraries to use compatible nullness code.

Hopefully, if a code use | null syntax it will still be able to be processed by F# even if nullable is not enabled otherwise libraries will probably need to add #if NULLABLE to offer both APIs...

[kerams]

Damn, that's a good point - we're compiling every library in a null-aware context.

Hopefully, if a code use | null syntax it will still be able to be processed by F#

Only with Fable 5 :/ Sadly, I don't think there's a way around this, and we'll just need to go update these projects.

[MangelMaxime]

Hopefully, if a code use | null syntax it will still be able to be processed by F#

Only with Fable 5 :/ Sadly, I don't think there's a way around this, and we'll just need to go update these projects.

Oh because otherwise, it will generate a syntax error right?

Unless, the library author use #if !NULLABLE or #if FABLE_COMPILER_4 to support non null able syntax.

This is unfortunate that this is causing issues in both direction...

[MangelMaxime]

If we wanted to avoid this problem, we would probably have to explore the idea of compiling the libraries and the main project in 2 separates projects.

Like mentioned here

I think we could have what is is needed to do that now, because Fable has the ability to use pre-compiled projects even if this is something I am not familiar with at all... And I am not sure how much work it would be to rework Fable to compile 2 projects instead of one.

Note: I believe I found a solution to support TreatWarningsAsErrors at Fable level without too much code change. Need to test it more

[MangelMaxime]

@kerams I made a release with support for Nullness and TreatWarningsAsErrors. Feel free to play with it and share your feedback in discussion/issues.

This is more or less experimental but the goal is to understand how this plays out in Fable, what are the requirements for the usage etc.

I am trying to port a real world project to Nullness to see how it plays out and make a Fable blog post for it.

[kerams]

Well, as expected, when I enable Nullable, I get errors from Fable packages, so the larger ecosystem needs to be updated before I can really enable it in my project.

[MangelMaxime]

@kerams Once, I have it working for one of my project I will make a Fable blog post trying to explain the situations.

So people can refer to it for asking libraries maintainers to make or accept PR adding support for nullness