Enhance Handling of F# Record Types as Props in ReactComponent Attribute

[lukaszkrzywizna]

Issue:

When using Feliz with F# and Fable, passing F# record types as props in React components results in unexpected behavior. The record type, which Fable translates into a JavaScript object with a specific prototype, loses its prototype once processed by React.createElement. This leads to the absence of expected methods and properties on the prototype.

Illustration of simple record transformation:

Differences in objects with and without the ReactComponent attribute:

Current Workaround by Users:

Currently, a practical workaround for users is to manually wrap the record into an anonymous object. This method involves encapsulating the record as a single property within an anonymous props object, which preserves the prototype through the React component lifecycle.

Example of wrapping the record:

Proposed Solution for Feliz:

I propose that Feliz should either:

1. Automatically wrap F# record types in an anonymous object when detected as props, thus preserving the prototype integrity.
2. Implement a warning or error system that alerts developers when a record type is used as props, guiding them towards safer practices or the recommended workaround.

@Zaid-Ajaj @MangelMaxime I'm interested in what you think.

[MangelMaxime]

@lukaszkrzywizna Just to be sure I am reading correctly, in the pictures below the normal functions is without the [<ReactComponent>] and what you would like to have all the time?

Differences in objects with and without the ReactComponent attribute:

Proposed Solution for Feliz:

I propose that Feliz should either:

1. Automatically wrap F# record types in an anonymous object when detected as props, thus preserving the prototype integrity.

I believe this solution would be the best as it requires no knowledge from the user. It would also mean that if F# says this API is available then it will works as expected etc.

However, we need to check if passing a pure Fable record works for react and don't have limitations. For example, will React still be able to diff the properties correctly even if they are 2 levels deep etc. Does this works with record decorated with [<AttachMembers>] which is an extreme case where the methods are attach to the prototype/class instead of being generated as a separated functions.

Regarding [<AttachMembers>], I believe the current implement does not works with it but because this issue is about keeping the prototype it made me think about this special case.

One problem, I could see with this changes is if people use F# to write their component and consume them from JS/TS. Then, they would need to build the properties using the Record constructor exposed by Fable to make sure they have the expected type instance.

[lukaszkrzywizna]

@lukaszkrzywizna Just to be sure I am reading correctly, in the pictures below the normal functions is without the [] and what you would like to have all the time?

A normal function means a function without the [<ReactComponent>] attribute. I would like to maintain the full record prototype chain because erasing it can cause hard-to-identify bugs. In my case, the record was used as a key for a map and it didn’t work due to the lack of that prototype.

However, we need to check if passing a pure Fable record works for react and don't have limitations.

I’ve prepared a new version with automatic wrapping, and so far everything works. It wraps only declared-type records (anonymous records are still processed in the same way). Check the #606.

Does this works with record decorated with [] which is an extreme case where the methods are attach to the prototype/class instead of being generated as a separated functions.

Thanks for the suggestion, I’ll check this.

[MangelMaxime]

Thinking about this issue and PR, there is actually 1 drawbacks in rewritings if the user use a record:

type MyProps =
	{
		value: string
		key: string
	}

Then the code seen by react with be:

createElement(Component, { props = { value: "", key: "" } })

This is problematic because now they component is not anymore optimised by with the key property for the diffing algorithm.

It looks like the current implementation is already making some assumptions on the key property interpretation when it is upper cased.

Feliz/Feliz.CompilerPlugins/ReactComponent.fs

Lines 81 to 84 in 4379c03

	// When the key property is upper-case (which is common in record fields)
	// then we should rewrite it
	let modifiedRecord = AstUtils.emitJs "(($value) => { $value.key = $value.Key; return $value; })($0)" [info.Args[0]]
	AstUtils.createElement reactElType [reactComponent; modifiedRecord]

I think to work around the issue, I pointed out we should also detect if the record has a key or Key property and if yes duplicate it at the top level too.

createElement(Component, { props = { value: "", key: "" }, key = "" })

This way we can keep both the Record prototype and the key diffing optimisation.

[lukaszkrzywizna]

@MangelMaxime I've added support for lowercased 'key' property.