-
wrapping components is repetetive, using all those @bs.deriving
- [problem] No one of us read the documentation and with it looking like a record we expected it to behave like a record.
- Why do we need to call
Composable.projectionConfigT
instead of just passing in a record? - Why is a type a function?
- Why do we need to call
- We should have checked the documentation, I just read it and now I understand it. In my opinion it should not look like a record if I cannot use it like one. Maybe the syntax can be changed. I don't know enough about PPX to know if that is possible though.
- [solution] Find another syntax that does not imply that this is a record.
- [solution] Do less autogeneration.
- [problem] No one of us read the documentation and with it looking like a record we expected it to behave like a record.
-
the interop has too much boilerplate
-
[problem] Every property has to be typed 3 times.
module ComposableMap = { [@bs.deriving abstract] type projectionConfigT = { scale: int, rotation: array(int), }; [@bs.deriving abstract] type jsProps = { projectionConfig: projectionConfigT, /* FIRST OCCURANCE */ width: int, height: int, style: ReactDOMRe.Style.t, }; [@bs.module "react-simple-maps"] external composableMap : ReasonReact.reactClass = "ComposableMap"; /* SECOND OCCURANCE */ let make = (~projectionConfig, ~width, ~height, ~style, children) => ReasonReact.wrapJsForReason( ~reactClass=composableMap, /* THIRD OCCURANCE */ ~props=jsProps(~projectionConfig, ~width, ~height, ~style), children, ); };
-
[solution] You want to define the wrapping more declaratively (maybe a PPX).
-
[solution]
ReasonReact.wrapJsForReason
should take props type and return a make function (not sure what it takes to get this working or if it is possible at all):module ComposableMap = { [@bs.deriving abstract] type projectionConfigT = { scale: int, rotation: array(int), }; [@bs.deriving abstract] type jsProps = { projectionConfig: projectionConfigT, width: int, height: int, style: ReactDOMRe.Style.t, }; [@bs.module "react-simple-maps"] external composableMap : ReasonReact.reactClass = "ComposableMap"; let make = ReasonReact.wrapJsForReason( ~reactClass=composableMap, ~props=jsProps, ); };
-
-
reactjs JSX and reason JSX should be copy 'n pastable
- People copy'n pasted the JSX from simple map example and expected it to work without changing a lot of syntax. I can ask for specifics if this is too general.
-
Belt's documentation could be improved
- Hard to find.
- You search on google.
- Find and empty git repository.
- Follow the link to bucklescript.
- See that autogenerated documentation which most people don't even realize is documentation.
- Standard format for documentation of reason/ocaml is hard to read, it looks so alien.
- People don't realize that you need to click module names to traverse it.
- Hard to find.
-
Using fastpipe for property access was confusing, it provided no syntactic improvement in that situation and confused people that where acqainted with the pipe operator: "Why use another pipe when the normal one would do the same job?"
/* These are all equal, why not just use the last one? */ let (lat, long) = marker |. Fetcher.location; let (lat, long) = marker |> Fetcher.location; let (lat, long) = Fetcher.location(marker);
-
SetState
is a bad name for an action. People didn't understand thatSetState
is a message type for the reduce and that those can be name arbitrarly by the programmer, they thought that just the way you dothis.setState()
in reason, and assuming that it das look convoluted and like overkill. -
open Belt
shadows the nativeList
andArray
module leading to weird issues where merlin tells you a function exists (specificallyArray.of_list
andArray.to_list
) but the compiler tells you it does not.- Shadowing variables/modules should give a warning in general.
open
should never be used in example code. It makes everyone's live miserable and usage should not be encouraged in the comunity 😄