jfmengels / elm-review-simplify

Provides elm-review rules to simplify your Elm code
https://package.elm-lang.org/packages/jfmengels/elm-review-simplify/latest/
BSD 3-Clause "New" or "Revised" License
20 stars 9 forks source link

General check and fix cleanup #237

Closed lue-bird closed 9 months ago

lue-bird commented 9 months ago

Note that unnecessaryCompositionAfterEmptyCheck enables new simplifications, e.g. Task.map f << Task.fail --> Task.fail which are deliberately not included in this PR.

Switching to Fn.Array.map will be the next PR

github-actions[bot] commented 9 months ago

The branch can be tried out by running:

elm-review --template jfmengels/elm-review-simplify/preview
jfmengels commented 9 months ago

LGTM so far (note for me, last commit I checked was Remove unused errorToLeft/RightRange)

lue-bird commented 9 months ago

Fighting https://github.com/elm/compiler/issues/2312, this somehow worked out now. However, I'm not confident this extensible record type alias extending TypeProperties thing won't cause headaches in the future.

E.g. setChecks has exactly one property nesting (the most intuitive one) which leads to a compiler while any other nesting compiles fine.

Sensible solutions might be to not alias TypeProperties in every other properties type alias or to completely switch to record fields for the properties. What do you say?

jfmengels commented 9 months ago

I'm fine with not having TypeProperties in every other properties type alias :+1: But feel free to switch to the other approach if you think it will work better :+1:

jfmengels commented 9 months ago

Great work :blush: