From bacf81d41bf0a51cee006fb0ca42e3f2d8f479e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Krzywizna?= Date: Thu, 23 May 2024 08:42:18 +0200 Subject: [PATCH 1/2] Added wrapping object for every type-declared record used as a props object with `[]` --- Feliz.CompilerPlugins/AstUtils.fs | 7 +- Feliz.CompilerPlugins/ReactComponent.fs | 88 +++++++++++++++++-------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/Feliz.CompilerPlugins/AstUtils.fs b/Feliz.CompilerPlugins/AstUtils.fs index 7d08f669..98ecf089 100644 --- a/Feliz.CompilerPlugins/AstUtils.fs +++ b/Feliz.CompilerPlugins/AstUtils.fs @@ -74,6 +74,11 @@ let makeImport (selector: string) (path: string) = Path = path.Trim() Kind = Fable.UserImport(false) }, Fable.Any, None) +let isDeclaredRecord (compiler: PluginHelper) (fableType: Fable.Type) = + match fableType with + | Fable.Type.DeclaredType (entity, genericArgs) -> compiler.GetEntity(entity).IsFSharpRecord + | _ -> false + let isRecord (compiler: PluginHelper) (fableType: Fable.Type) = match fableType with | Fable.Type.AnonymousRecordType _ -> true @@ -163,4 +168,4 @@ let capitalize (input: string) = let camelCase (input: string) = if String.IsNullOrWhiteSpace input then "" - else input.First().ToString().ToLower() + String.Join("", input.Skip(1)) + else input.First().ToString().ToLower() + String.Join("", input.Skip(1)) \ No newline at end of file diff --git a/Feliz.CompilerPlugins/ReactComponent.fs b/Feliz.CompilerPlugins/ReactComponent.fs index 4ad4675b..5fa7cfb6 100644 --- a/Feliz.CompilerPlugins/ReactComponent.fs +++ b/Feliz.CompilerPlugins/ReactComponent.fs @@ -49,6 +49,30 @@ module internal ReactComponentHelpers = { decl with MemberRef = info; Args = []; Body = body } | _ -> { decl with Body = injectReactImport decl.Body } + + let rewriteArgs (decl: MemberDecl) = + // rewrite all other arguments into getters of a single props object + // TODO: transform any callback into into useCallback(callback) to stabilize reference + let propsArg = AstUtils.makeIdent (sprintf "%sInputProps" (AstUtils.camelCase decl.Name)) + let propBindings = + ([], decl.Args) ||> List.fold (fun bindings arg -> + let getterKey = if arg.DisplayName = "key" then "$key" else arg.DisplayName + let getterKind = ExprGet(AstUtils.makeStrConst getterKey) + let getter = Get(IdentExpr propsArg, getterKind, Any, None) + (arg, getter)::bindings) + |> List.rev + + let body = + match decl.Body with + // If the body is surrounded by a memo call we put the bindings within the call + // because Fable will later move the surrounding function into memo + | Call(ReactMemo reactMemo, ({ Args = arg::restArgs } as callInfo), t, r) -> + let arg = propBindings |> List.fold (fun body (k,v) -> Let(k, v, body)) arg + Call(reactMemo, { callInfo with Args = arg::restArgs }, t, r) + | _ -> + propBindings |> List.fold (fun body (k,v) -> Let(k, v, body)) decl.Body + + { decl with Args = [propsArg]; Body = body } open ReactComponentHelpers @@ -74,16 +98,39 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string callee if List.length membArgs = info.Args.Length && info.Args.Length = 1 && AstUtils.isRecord compiler info.Args[0].Type then + + // declared record + // https://github.com/Zaid-Ajaj/Feliz/issues/603 + // F# Component { Value = 1 } + // JSX + // JS createElement(Component, { props = { Value: 1 } }) + + // anonymous record // F# Component { Value = 1 } // JSX // JS createElement(Component, { Value: 1 }) + + let isDeclaredRecord = AstUtils.isDeclaredRecord compiler info.Args[0].Type + if AstUtils.recordHasField "Key" compiler info.Args[0].Type then // 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]] + let modifiedRecord = + if isDeclaredRecord then + AstUtils.objExpr [ + "key", AstUtils.emitJs "$0.Key" [info.Args[0]]; + membArgs[0].Name.Value, info.Args[0] + ] + else + AstUtils.emitJs "(($value) => { $value.key = $value.Key; return $value; })($0)" [info.Args[0]] AstUtils.createElement reactElType [reactComponent; modifiedRecord] else - AstUtils.createElement reactElType [reactComponent; info.Args[0]] + let value = + if isDeclaredRecord then + AstUtils.objExpr [ membArgs[0].Name.Value, info.Args[0] ] + else + info.Args[0] + AstUtils.createElement reactElType [reactComponent; value] elif info.Args.Length = 1 && info.Args[0].Type = Type.Unit then // F# Component() // JSX @@ -93,7 +140,8 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string let mutable keyBinding = None let propsObj = - List.zip (List.take info.Args.Length membArgs) info.Args + info.Args + |> List.zip (List.take info.Args.Length membArgs) |> List.collect (fun (arg, expr) -> match arg.Name, expr with | Some "key", IdentExpr _ -> ["key", expr; "$key", expr] @@ -144,8 +192,13 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string | Some true -> { decl with Tags = "export-default"::decl.Tags } | Some false | None -> decl - // do not rewrite components accepting records as input - if decl.Args.Length = 1 && AstUtils.isRecord compiler decl.Args[0].Type then + // do not rewrite components accepting anonymous records as input + if decl.Args.Length = 1 && AstUtils.isAnonymousRecord decl.Args.[0].Type then + decl + |> applyImportOrMemo import from memo + // put record into a single props object to stabilize prototype chain + // https://github.com/Zaid-Ajaj/Feliz/issues/603 + elif decl.Args.Length = 1 && AstUtils.isDeclaredRecord compiler decl.Args[0].Type then // check whether the record type is defined in this file // trigger warning if that is case let definedInThisFile = @@ -186,34 +239,15 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string () decl + |> rewriteArgs |> applyImportOrMemo import from memo else if decl.Args.Length = 1 && decl.Args[0].Type = Type.Unit then // remove arguments from functions requiring unit as input { decl with Args = [ ] } |> applyImportOrMemo import from memo else - // rewrite all other arguments into getters of a single props object - // TODO: transform any callback into into useCallback(callback) to stabilize reference - let propsArg = AstUtils.makeIdent (sprintf "%sInputProps" (AstUtils.camelCase decl.Name)) - let propBindings = - ([], decl.Args) ||> List.fold (fun bindings arg -> - let getterKey = if arg.DisplayName = "key" then "$key" else arg.DisplayName - let getterKind = ExprGet(AstUtils.makeStrConst getterKey) - let getter = Get(IdentExpr propsArg, getterKind, Any, None) - (arg, getter)::bindings) - |> List.rev - - let body = - match decl.Body with - // If the body is surrounded by a memo call we put the bindings within the call - // because Fable will later move the surrounding function into memo - | Call(ReactMemo reactMemo, ({ Args = arg::restArgs } as callInfo), t, r) -> - let arg = propBindings |> List.fold (fun body (k,v) -> Let(k, v, body)) arg - Call(reactMemo, { callInfo with Args = arg::restArgs }, t, r) - | _ -> - propBindings |> List.fold (fun body (k,v) -> Let(k, v, body)) decl.Body - - { decl with Args = [propsArg]; Body = body } + decl + |> rewriteArgs |> applyImportOrMemo import from memo type ReactMemoComponentAttribute(?exportDefault: bool) = From c671a292fe6db5274164f70b76b07740f82b562c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Krzywizna?= Date: Fri, 7 Jun 2024 15:27:23 +0200 Subject: [PATCH 2/2] ReactComponent attribute: - Added support for records with a lowercased 'key' property. - Added warning for anonymous records used as props-object with a lowercased 'key' property. --- Feliz.CompilerPlugins/AstUtils.fs | 10 +++++---- Feliz.CompilerPlugins/ReactComponent.fs | 30 +++++++++++++++---------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/Feliz.CompilerPlugins/AstUtils.fs b/Feliz.CompilerPlugins/AstUtils.fs index 98ecf089..d88cfd5e 100644 --- a/Feliz.CompilerPlugins/AstUtils.fs +++ b/Feliz.CompilerPlugins/AstUtils.fs @@ -106,18 +106,20 @@ let isReactElement (fableType: Fable.Type) = | Fable.Type.DeclaredType (entity, genericArgs) -> entity.FullName.EndsWith "ReactElement" | _ -> false -let recordHasField name (compiler: PluginHelper) (fableType: Fable.Type) = +let tryGetRecordField (name: string) (compiler: PluginHelper) (fableType: Fable.Type) = + let name = name.ToLower() match fableType with | Fable.Type.AnonymousRecordType (fieldNames, genericArgs, _isStruct) -> fieldNames - |> Array.exists (fun field -> field = name) + |> Array.tryFind (fun field -> field.ToLower() = name) | Fable.Type.DeclaredType (entity, genericArgs) -> compiler.GetEntity(entity).FSharpFields - |> List.exists (fun field -> field.Name = name) + |> List.tryFind (fun field -> field.Name.ToLower() = name) + |> Option.map(fun field -> field.Name) | _ -> - false + None let memberName = function | Fable.MemberRef(_,m) -> m.CompiledName diff --git a/Feliz.CompilerPlugins/ReactComponent.fs b/Feliz.CompilerPlugins/ReactComponent.fs index 5fa7cfb6..ac972927 100644 --- a/Feliz.CompilerPlugins/ReactComponent.fs +++ b/Feliz.CompilerPlugins/ReactComponent.fs @@ -102,29 +102,30 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string // declared record // https://github.com/Zaid-Ajaj/Feliz/issues/603 // F# Component { Value = 1 } - // JSX - // JS createElement(Component, { props = { Value: 1 } }) + // JSX + // JS createElement(Component, props = { Value: 1 }) // anonymous record - // F# Component { Value = 1 } + // F# Component {| Value = 1 |} // JSX // JS createElement(Component, { Value: 1 }) let isDeclaredRecord = AstUtils.isDeclaredRecord compiler info.Args[0].Type - if AstUtils.recordHasField "Key" compiler info.Args[0].Type then + match AstUtils.tryGetRecordField "key" compiler info.Args[0].Type with + | Some keyField when isDeclaredRecord -> // When the key property is upper-case (which is common in record fields) // then we should rewrite it let modifiedRecord = - if isDeclaredRecord then - AstUtils.objExpr [ - "key", AstUtils.emitJs "$0.Key" [info.Args[0]]; - membArgs[0].Name.Value, info.Args[0] - ] - else - AstUtils.emitJs "(($value) => { $value.key = $value.Key; return $value; })($0)" [info.Args[0]] + AstUtils.emitJs + (sprintf "(($value) => { $value.key = $value.%s.%s; return $value; })($0)" (membArgs[0].Name.Value) keyField) + [ AstUtils.objExpr [ membArgs[0].Name.Value, info.Args[0]] ] AstUtils.createElement reactElType [reactComponent; modifiedRecord] - else + | Some "Key" -> // anonymous record won't have wrapped object so 'key' (lowercase) prop is automatically recognized + let modifiedRecord = + AstUtils.emitJs "(($value) => { $value.key = $value.Key; return $value; })($0)" [info.Args[0]] + AstUtils.createElement reactElType [reactComponent; modifiedRecord] + | _ -> let value = if isDeclaredRecord then AstUtils.objExpr [ membArgs[0].Name.Value, info.Args[0] ] @@ -194,6 +195,11 @@ type ReactComponentAttribute(?exportDefault: bool, ?import: string, ?from:string // do not rewrite components accepting anonymous records as input if decl.Args.Length = 1 && AstUtils.isAnonymousRecord decl.Args.[0].Type then + if AstUtils.tryGetRecordField "key" compiler decl.Args[0].Type = Some "key" then + let errorMessage = + sprintf "The function %s expects an anonymous record with a 'key' property, which is not allowed by React. The value will be erased and it will return undefined. More info: https://reactjs.org/link/special-props" decl.Name + compiler.LogWarning(errorMessage, ?range=decl.Body.Range) + decl |> applyImportOrMemo import from memo // put record into a single props object to stabilize prototype chain