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

Function decorated with [<NamedParams>] fails at runtime if all the arguments are optional and none are provided #3480

Closed
Freymaurer opened this issue Jun 27, 2023 · 8 comments

Comments

@Freymaurer
Copy link
Contributor

Description

Hey i am working on a library we want to offer for js as well as f#. So to improve JS syntax we wanted to use [<NamedParams>] on members.
I got a very strange error of:
image

.. when running my Fable.Mocha tests. To add to this:

  • If i use [<NamedParams(fromIndex=0)>] or [<NamedParams>] it runs for Example 1.
  • Example 2 raises a similiar error no matter if i use (fromIndex) or not. So i would suspect the issue being ?param f# syntax
  • Fable Repl replicating Example 2 runs.

Repro code

Fable Repl

link

Example 1:

Library source code

Example 1:

https://github.com/nfdi4plants/ISADotNet/blob/ea4e1cefdceb8b0899f029409ee5ac4b003268f5/src/ISA/ISA/ArcTypes/ArcTable.fs#L166

Example 1:

https://github.com/nfdi4plants/ISADotNet/blob/ea4e1cefdceb8b0899f029409ee5ac4b003268f5/src/ISA/ISA/ArcTypes/ArcAssay.fs#L108

Expected and actual results

I would expect NamedParams to work in this case. Not sure where something is going wrong

Related information

  • Fable version: dotnet fable --version: 4.1.4
  • Operating system: Windows 11
@MangelMaxime
Copy link
Member

Hello @Freymaurer,

If you look at the generated JavaScript, the problem comes from this line export const assay = ArcAssay.create();

When calling ArcAssay.create() without providing any arguments, Fable doesn't generate an empty {}.

Indeed, without provide {} to the create function, it means that the code is trying to destructure undefined which leads to the error you are seeing.


I would also like to ask you this question:

What is your goal here for using [<NamedParams>]?

The only usage that I know of [<NamedParams>] and [<ParamObject>] are for bindings right now.

But here, it seems like you are mixing it with some actual code and I don't know if this is an officially supported usage.

In your case, can't you use the default F# method generation, without having the JavaScript modified to use destructuring patterns?

@Freymaurer
Copy link
Contributor Author

I want to use [<NamedParams>] to avoid passing void if not all params are given.

// Without NamedParams
export const assay = ArcAssay.create();

export const assay2 = ArcAssay.create("Hello", void 0, "MyType");
// With NamedParams
export const assay = ArcAssay.create();

export const assay2 = ArcAssay.create({
    id: "Hello",
    measurementType: "MyType",
});

Both taken from Fable.Repl

@MangelMaxime
Copy link
Member

Ok, we will have to wait other maintainer opinion on this one.

NamedParams and ParamObject are a new addition made by @alfonsogarciacaro. I don't know what is position is on this subject.

@MangelMaxime MangelMaxime changed the title [<NamedParams>] throwing unexpected error Function decorated with [<NamedParams>] fails at runtime if all the arguments are optional and none are provided Jun 27, 2023
@Freymaurer
Copy link
Contributor Author

Update for the REPL. It does in fact not run correctly. I tried printing the created assays to the console and it does not work, unless i remove the ArcAssay.create(). So it also seems to error, without recognizing it errored.

@MangelMaxime
Copy link
Member

Yes ArcAssay.create() doesn't work.

This is because, it generates ArcAssay.create() instead of ArcAssay.create({}).

Meaning that there are no object to desconstruct.

If you provide at least 1 argument, then this works because the object will be initialised.

@Freymaurer
Copy link
Contributor Author

Ok, we will have to wait other maintainer opinion on this one.

NamedParams and ParamObject are a new addition made by @alfonsogarciacaro. I don't know what is position is on this subject.

Any new information on this? 😄 This would be a very nice option for libraries which should be usable on multiple languages.

MangelMaxime pushed a commit that referenced this issue Aug 9, 2023
@MangelMaxime
Copy link
Member

@alfonsogarciacaro @ncave

In order to fix this issue, I had make changes in Fable.AST

Does it means we need to ask for a re-release of Feliz.CompilerPlugins ?

@DunetsNM
Copy link
Contributor

addition of Attributes : Attribute seq to MemberRefInfo broke json deserialization in Util.fs :

Json.readWithStringPool<(string * Fable.InlineExpr)

it now throws

error EXCEPTION: Deserialization of interface types is not supported. Type 'Fable.AST.Fable.Attribute'. Path: $[0] | LineNumber: 0 | BytePositionInLine: 2.
at System.Text.Json.ThrowHelper.ThrowNotSupportedException(ReadStack& state, Utf8JsonReader& reader, NotSupportedException ex)

I don't have a small repro just yet - will add a separate issue when I have it (seems to be related to using --precompiledLib flag) , but root cause is pretty obvious: Attribute is an interface which also has ConstructorArgs: obj list i.e. no chance it can serialize.

It fails even though in my inline_exprs_0.json file all Attributes collections are empty. Good news it has only one usage: literally to check presence of "Fable.Core.ParamObjectAttribute", so it's easy to fix by replacing Attributes: Attribute seq with AttributeNames: string seq or even HasParamObjectAttribute: bool

Lesson to extract here is to keep coupling between types at minimum

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

3 participants