-
Notifications
You must be signed in to change notification settings - Fork 31
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
Bump PureScript to 0.14.3 #280
Bump PureScript to 0.14.3 #280
Conversation
Thanks for working on this! |
6594118
to
18fe681
Compare
exampleJson :: Json | ||
exampleJson = unsafePartial $ fromRight $ jsonParser | ||
exampleJson = unsafePartial $ (\(Right json) -> json) $ jsonParser |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fromRight
is no longer a partial function https://pursuit.purescript.org/packages/purescript-either/5.0.0/docs/Data.Either#v:fromRight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered making a standalone function for this, something like
fromRight :: Partial => forall a b. Either a b -> b
fromRight (Right x) = x
but I opted for the minimal change. I'm definitely open to suggestions here; this one in particular felt like one of the more questionable changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people have typically been doing something like either (\_ -> unsafeCrashWith "got Left: bad parser") identity
. It's a bit longer, but then it at least provides a somewhat clearer error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
case readFloat fullString of | ||
x | isFinite x -> pure $ LitNum x | ||
case Number.fromString fullString of | ||
Just x | Number.isFinite x -> pure $ LitNum x |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't find the partial readFloat
here, so replaced with fromString
. Since there was already a fromString
imported from Data.Int
, I had to namespace both imports to disambiguate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry about these ones:
Can't do anything about these ones other than get the packages added again to the package set:
|
Sounds good 👍 those seem reasonable enough to tackle in this PR |
Thanks again for all this work! |
type LOGGER = FProxy LoggerF | ||
type GET_USER_NAME = FProxy GetUserNameF | ||
type LOGGER r = (logger :: LoggerF | r) | ||
type GET_USER_NAME r = (getUserName :: GetUserNameF | r) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This compiles and works, but I was just guessing my way through this refactor while referencing the Run
docs. There might be an alternative that's closer to the original.
recipes/SignalTrisJs/src/Main.purs
Outdated
----------------- | ||
-- Faster versions of these functions are proposed in | ||
-- https://github.com/purescript/purescript-ordered-collections/pull/21 | ||
toMap :: forall a. Ord a => Set a -> Map a Unit | ||
toMap = foldl (\m k -> Map.insert k unit m) mempty | ||
|
||
annotateSet :: forall a b. Ord a => (a -> b) -> Set a -> Map a b | ||
annotateSet f = mapWithIndex (\k _ -> f k) <<< toMap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toMap
is now in Data.Set
, so no need for a Monoid
instance here. I couldn't find anything like annotateSet
in there though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just added annotateSet
to the todo list in purescript/purescript-ordered-collections#50
Might be good to link to that new issue in a comment here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"purescript": "^0.13.8", | ||
"spago": "^0.16.0" | ||
"purescript": "^0.14.3", | ||
"spago": "^0.19.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll want to bump this to the latest spago version, but that should be done in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to make before merging this:
- Let's change migrate from the
fromRight
partial function toeither (\_ -> unsafeCrashWith "got Left: bad parser") identity
fromRight' (\_ -> unsafeCrashWith "got Left: bad parser")
. - Check whether
v0.14.3
compiles this PR too (if it does, use it; if not, ignore it and continue with the one you have).
Thank you for all your work!
I think either (\_ -> unsafeCrashWith "got Left: bad parser") identity is same as fromRight' \_ -> unsafeCrashWith "got Left: bad parser" I'm fine with either (no pun intended) but would you still prefer the former? |
Oh, whoops! Yeah, let's use the |
|
I'm going to use a merge commit for this PR because it touches so many different recipes. |
All told, only
1714 of 80 ended up in thebroken
pile which doesn't seem too bad.❌ Move to
broken
✏️ Add missing dependencies to
spago.dhall
❌ Move to
broken
✏️ Add missing dependencies to
spago.dhall
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
❌ Move to
broken
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✏️ Change
NonEmpty Array
toNonEmptyArray
✏️ Change
NonEmpty Array
toNonEmptyArray
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✅ Already works
❌ Move to
broken
❌ Move to
broken
✅ Already works
❌ Move to
broken
✅ Already works
✅ Already works
✅ Already works
✏️ Change
Debug.Trace
toDebug
✏️ Switch argument order
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
❌ Move to
broken
❌ Move to
broken
❌ Move to
broken
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✅ Already works
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️
appendChild
returnsUnit
instead ofNode
✅ Already works
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✅ Already works
✅ Already works
✅ Already works
✏️ Replace
SProxy
withProxy
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
❌ Move to
broken
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✏️ Replace
SProxy
withProxy
❌ Move to
broken
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✅ Already works
✅ Already works
✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✅ Already works
✅ Already works
✅ Already works
❌ Move to
broken
✏️ Remove
generics-rep
fromspago.dhall
✏️ Replace
Data.Generic.Rep.Show
withData.Show.Generic
✏️ Remove
generics-rep
fromspago.dhall
❌ Move to
broken
✏️ Remove
generics-rep
fromspago.dhall
✏️ Update usage of
Run
✏️ Update to reflect a few breaking changes in
Halogen.Svg.Attributes
✅ Already works
✅ Already works
✅ Already works
✏️ Use
toMap
fromData.Set
✏️ Remove
toNonEmpty
✏️ Replace
Global
import with qualifiedData.Number
import✏️ Remove
HH.HTML
as parameter toComponent
✏️ Update event handler signature from
MouseEvent -> Maybe i
toMouseEvent -> i
✅ Already works
✏️ Remove
generics-rep
fromspago.dhall
✏️ Replace
Data.Generic.Rep.Show
withData.Show.Generic
❌ Move to
broken
✅ Already works
✏️ Replace partial
fromRight
with inline lambda✅ Already works
✅ Already works