reanalyze icon indicating copy to clipboard operation
reanalyze copied to clipboard

[Feature request] Integrate with `@deriving({abstract: light})`

Open mvaled opened this issue 2 years ago • 13 comments

All the fields in the following script are reported as dead:

@deriving({abstract: light})
type person = {
  name: string,
  age: int,
}

let joe = person(~name="Joe", ~age=20)
let joeName = name(joe)

Report:

  Warning Dead Type
  File "src/TestBug.res", line 3, characters 2-14
  person.name is a record label never used to read a value
  <-- line 3
    @dead("person.name") name: string,

  Warning Dead Type
  File "src/TestBug.res", line 4, characters 2-10
  person.age is a record label never used to read a value
  <-- line 4
    @dead("person.age") age: int,

However, only age is not actually used in the code, while name shouldn't be reported.

mvaled avatar Sep 27 '22 09:09 mvaled

Deriving abstract is expected to be used less and less in future. Either objects, or records, in the latest compiler versions, should be able to express all the needed functionality.

cristianoc avatar Sep 27 '22 10:09 cristianoc

We use it heavily in a custom integration we have of a React-like framework based on Mithril -- this is mainly because our app is offline-first and the connection is very limited; so we wanted a really small library.

This, is for instance a very small component:

open Prelude
open Mithril
open CompletionStatus

@deriving({abstract: light})
type makeProps = {
  @optional completion: completionInformation,
  @optional children: element,
}

let make = _ => {
  component()->view(vnode => {
    let completion = vnode.attrs->completion->default(CompletionStatus.Defaults.completion)
    let {totalNested, totalInspected, isOverdue} = completion

    <div className="tile-action">
      {show(
        if completion->isFullyInspected {
          <Feather icon=#check_circle className="text-success" />
        } else {
          <span className={isOverdue ? "text-error" : ""}>
            {show(totalInspected->Int.toString ++ "/" ++ totalNested->Int.toString)}
          </span>
        },
      )}
    </div>
  })
}

And it can be used like <ShowCompletionInfo completion={...}/>.


It would require some rethinking to remove the dependency on abstract.

mvaled avatar Sep 27 '22 11:09 mvaled

@mattdamon108 this seems doable with JSX V4?

cristianoc avatar Sep 27 '22 11:09 cristianoc

@mattdamon108 this seems doable with JSX V4?

Yes, I think so too.

@mvaled Can you try the latest compiler and rescript-react as posted on the forum? https://forum.rescript-lang.org/t/call-for-help-2-test-jsx-v4/3781 Maybe something like:

type props<'completion, 'children> = {
  completion: 'completion,
  children: 'children
}
let make = props => {
  let {completion, children} = props

  component()->view(vnode => {
    let completion = vnode.attrs->completion->default(CompletionStatus.Defaults.completion)
    let {totalNested, totalInspected, isOverdue} = completion

    <div className="tile-action">
      {show(
        if completion->isFullyInspected {
          <Feather icon=#check_circle className="text-success" />
        } else {
          <span className={isOverdue ? "text-error" : ""}>
            {show(totalInspected->Int.toString ++ "/" ++ totalNested->Int.toString)}
          </span>
        },
      )}
    </div>
  })
}

mununki avatar Sep 27 '22 11:09 mununki

If it is not working then try this:

type props = {
  completion: completionInformation,
  children: element
}

mununki avatar Sep 27 '22 11:09 mununki

@mattdamon108

@mvaled Can you try the latest compiler and rescript-react as posted on the forum? https://forum.rescript-lang.org/t/call-for-help-2-test-jsx-v4/3781

Thanks.

One question, though. Would this require the actual React JS library or it's just the transformation, that would work with any compatible "react" binding? e.g Our rescript-mithril instead of '@rescript/react'.

If it's just the transformation, then I'll try to upgrade our Mithril bindings to be compatible with JSX v4.

mvaled avatar Sep 27 '22 11:09 mvaled

I just checked about Mithril, never knew before. You made your own bindings! How about your bsconfig.json? Do you use "reason"."react-jsx" option?

mununki avatar Sep 27 '22 11:09 mununki

I've looked into your bindings. I think it would be working highly possibility. You can check the latest rescript-react binding and update your Mithrill bindings accordingly, then try with latest compiler's JSX v4.

mununki avatar Sep 27 '22 11:09 mununki

One question, though. Would this require the actual React JS library or it's just the transformation, that would work with any compatible "react" binding?

Just transformation and working with react binding.

mununki avatar Sep 27 '22 11:09 mununki

How about your bsconfig.json? Do you use "reason"."react-jsx" option?

None, actually. This is our whole bsconfig.json:

{
  "name": "kaiko-survey-tool",
  "sources": [
    {
      "dir": "src/",
      "subdirs": true
    }
  ],
  "suffix": ".js",
  "reason": {
    "react-jsx": 3
  },
  "bs-dependencies": [
    "@kaiko/reindexed",
    "rescript-webapi",
    "@ryyppy/rescript-promise",
    "rescript-mithril",
    "bs-fetch",
    "rescript-prelude",
    "rescript-deser",
    "rescript-action-queue"
  ],
  "package-specs": {
    "module": "es6"
  },
  "warnings": {
    "error": "+8+11+26+27+33+56"
  }
}

mvaled avatar Sep 27 '22 12:09 mvaled

bsconfig seems you're using jsx transformation already. Yes, it's good sign. Good luck!

mununki avatar Sep 27 '22 12:09 mununki

When you try the latest compiler with JSX v4, change this property in bsconfig.json

"reason": {
    "react-jsx": 3
  },

to

"jsx": {
  "version": 4
}

mununki avatar Sep 27 '22 12:09 mununki

bsconfig seems you're using jsx transformation already

Right! Somehow my eyes skip over those lines 😄

mvaled avatar Sep 27 '22 12:09 mvaled