-
Notifications
You must be signed in to change notification settings - Fork 19
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
Make parse
safer
#15
Comments
I agree with having more safe standard functions. 👍 foreign import unsafeParse :: forall eff. String -> Eff (locale :: LOCALE | eff) JSDate
parse :: String -> Eff _ (Maybe JSDate)
-- or --
foreign import parseFFI :: forall a. Maybe a -> (a -> Maybe a) -> String -> Eff _ (Maybe JSDate)
parse :: String -> Eff _ (Maybe JSDate)
parse = parseFFI Nothing Just
-- or --
parse : String -> Eff _ (Maybe JSDate)
parse = do
parsed <- unsafeParse date
pure $ if isValid parsed then Just parsed else Nothing |
I think we should really be discouraging the use of However, I think it does make sense to include AFAIK, this package is intended to be nothing more than a thin wrapper over the JS API, and so I think the current behaviour does actually make the most sense. I'd like to update the docs to discourage the use of |
I've labeled this as
|
@thomashoneyman Would you envision this as function-level documentation on |
I think the contents of the README adequately say what this package is for already, so I think it should just be a case of adding some clarity to the function-level docs for |
I agree with @hdgarrood. The README already specifies what the general package is for (and recommends alternatives), so it's the function itself that ought to have a bit more documentation. |
We've addressed this in #25. |
I was originally doing the following and easily ran into a runtime error because
isoString
was invalid:I'm currently doing this instead:
Is there any reason to not make
parse
do this by default?The text was updated successfully, but these errors were encountered: