darklang / rescript-tea

The Elm Architecture for Rescript
Other
119 stars 9 forks source link

Switch to rescript webapi #26

Closed OceanOak closed 2 years ago

OceanOak commented 2 years ago

Changes made :

tea_ex.res Some functions used to return option type, their equivalents in Rescript API return "unit" which we can't pass in a switch statement. I instead used if-else expressions. Here is an example of how it was used

    let removeItem = key =>
    nativeBinding(cb =>{
      let value=Dom.Storage.removeItem(key, Dom.Storage.localStorage)
      if(Js.testAny( value)) {
      cb(Error("localStorage is not available"))}
      else{
        cb(Ok(value))}})

I followed the same process in the following functions:

Is there a better way to do this?

tea_animationframe.res

tea_time.res Instead of Web.Window's "setInterval" and "clearTimeout", I used Js.Global module "setIntervalFloat" and "clearInterval"

Note: There are still other files that need to be switched, currentely working on them.

pbiggar commented 2 years ago

let removeItem = key => nativeBinding(cb =>{ let value=Dom.Storage.removeItem(key, Dom.Storage.localStorage) if(Js.testAny( value)) { cb(Error("localStorage is not available"))} else{ cb(Ok(value))}})

It looks like localstorage is pretty pervasive: it's got 97% on canIuse.

If local storage was unavailable, would this be undefined or throw an exception? If undefined, maybe test for that directly?

OceanOak commented 2 years ago

Running an availability test in the Js file throws this exception if the localstorage is not available: localStorage is not defined

Testing it with undefined (Js.Nullable.undefined or Js.undefined) in the .res file throws an error. This has type: Js.undefined<'a> or This has type: Js.Nullable.t<'a> Somewhere wanted: Dom.Storage.t

I hope this answers your question. If it doesn't let me know.

pbiggar commented 2 years ago

If it throws an exception, then I guess we should catch the exception instead of comparing it. Right?

OceanOak commented 2 years ago

Is that better?

pbiggar commented 2 years ago

Needs to be try/catch: https://rescript-lang.org/docs/manual/latest/exception

OceanOak commented 2 years ago

I have tried using try/catch here is an example:

  let removeItemCmd = key => Tea_task.attemptOpt(_ => None, removeItem(key))
  let setItem = (key, value) =>
    nativeBinding(cb =>{
      try Dom.Storage.setItem(key, value, Dom.Storage.localStorage)
      catch{
        | Not_found=> cb(Error("localStorage is not available"))
      }}
    )

It works for all the functions except for "length" function because it returns an integer and "try" needs a unit type. That is why I used the second way of handling exeptions which I found in this manual. Should I use try/catch for all of them and leave "length" as it is or is there a way to use try/catch for it ?

pbiggar commented 2 years ago

It works for all the functions except for "length" function because it returns an integer and "try" needs a unit type.

It's not that try needs a unit type, it's not try and catch need to have the same time. Something like this:

let length = nativeBinding(cb =>{
      try {
        cb(Ok(Dom.Storage.length(Dom.Storage.localStorage)))
       } catch {
      | exception Not_found =>cb(Error("localStorage is not available"))
    }}
  )
OceanOak commented 2 years ago

It works for all the functions except for "length" function because it returns an integer and "try" needs a unit type.

It's not that try needs a unit type, it's not try and catch need to have the same time. Something like this:

let length = nativeBinding(cb =>{
      try {
        cb(Ok(Dom.Storage.length(Dom.Storage.localStorage)))
       } catch {
      | exception Not_found =>cb(Error("localStorage is not available"))
    }}
  )

I really don't know why I removed cb(Ok(...)) in the first place. Sorry for that and thanks for the information.