Switch to rescript webapi

[OceanOak]

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:

• length
• clear
• removeItem
• setItem

Is there a better way to do this?

tea_animationframe.res

• Used Js.Date instead of Web.Date
• Used Webapi's "requestCancellableAnimationFrame and "cancelAnimationFrame" instead of Web.Window 's "requestAnimationFrame" and "cancelAnimationFrame".

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]

This looks great! A few questions, and we should be good to merge.

[pbiggar]

I think it would be better to not open this module, and just use a qualified in each place instead.

[OceanOak]

Changes are done.

[pbiggar]

Again, I think it would be better to skip the opens

[OceanOak]

Changes are done.

[pbiggar]

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]

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]

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

[OceanOak]

Is that better?

[pbiggar]

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

[OceanOak]

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]

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]

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.

[pbiggar]

Nearly there, thanks!

[pbiggar]

It's not clear to me why this is Not_found. I think any exception should go here.

[pbiggar]

Same for all of these

[OceanOak]

Do we know what other exceptions do we need to catch ?
Do we need to catch
| Invalid_argument(_) =>...
( if yes, should I change the error message?)
Do we need to catch JS Exceptions using :
| Js.Exn.Error(obj)
even though they don't encourage that in the documentation.
Or is there a better way to catch all exceptions without writing them one by one?
I don't know if it is correct to do something like that
| _e => cb(Error("localStorage is not available"))

[pbiggar]

| _e => cb(Error("localStorage is not available")) - yeah, exactly

[pbiggar]

This should use try

[pbiggar]

This should use try