new [@react.component] vs .rei #3519

diligiant · 2019-04-18T08:35:25Z

While updating components to 5.0.4 (used master ##bb21ecb) and v3 syntax. I hit the following error :

The implementation src/Helmet.mlast
       does not match the interface src/helmet-WaykeHelmet.cmi:
       Values do not match:
         [@bs...] external makeProps:
           (~title: 'title=?, ~defaultTitle: 'defaultTitle=?, ~meta: 'meta=?,
           ~htmlAttributes: 'htmlAttributes=?, ~children: 'children,
           ~key: string=?, unit) =>
           {. "children": 'children, "defaultTitle": option('defaultTitle),
             "htmlAttributes": option('htmlAttributes),
             "meta": option('meta), "title": option('title)}
           = ""
       is not included in
         [@bs...] external makeProps:
           (~title: string=?, ~defaultTitle: string=?,
           ~meta: array(metaField)=?,
           ~htmlAttributes: array(htmlAttribute)=?, ~children: React.element,
           ~key: string=?, unit) =>
           {. "children": React.element, "defaultTitle": option(string),
             "htmlAttributes": option(array(htmlAttribute)),
             "meta": option(array(metaField)), "title": option(string)}
           = ""
       File "/.../src/Helmet.re", line 33, characters 2-17: Actual declaration

@rickyvetter made me check that it's not a ppx issue. files are below.

.re

let make = (~title=?, ~defaultTitle=?, ~meta=?, ~htmlAttributes=?, ~children) => {…}

.rei

let make: (~title: string=?, ~defaultTitle: string=?, ~meta: array(metaField)=?, ~htmlAttributes: array(htmlAttribute)=?, ~children: React.element) => React.element;```

The text was updated successfully, but these errors were encountered:

bobzhang · 2019-04-18T12:05:15Z

do you have a small example, this seems to be a ppx issue. can you try add type annotations to make see if it is indeed the type

diligiant · 2019-04-18T13:35:08Z

@bobzhang is the following more useful?
H.re

type metaField =
  | MetaName(string, string)
  | MetaProperty(string, string);

type htmlAttribute =
  | Lang(string)
  | Amp
  | Other(string, string);

[@react.component]
let make = (~title: string=?,
    ~defaultTitle: string=?,
    ~meta: array(metaField)=?,
    ~htmlAttributes: array(htmlAttribute)=?,
    ~children: React.element): React.element => {
      <div />
};

H.rei

type metaField =
  | MetaName(string, string)
  | MetaProperty(string, string);

type htmlAttribute =
  | Lang(string)
  | Amp
  | Other(string, string);

[@react.component]
let make:
  (
    ~title: string=?,
    ~defaultTitle: string=?,
    ~meta: array(metaField)=?,
    ~htmlAttributes: array(htmlAttribute)=?,
    ~children: React.element
  ) =>
  React.element;

-make-world =>

 The implementation src/H.mlast
       does not match the interface src/h-WaykeHelmet.cmi:
       Values do not match:
         [@bs...] external makeProps:
           (~title: string=?, ~defaultTitle: string=?,
           ~meta: array(metaField)=?,
           ~htmlAttributes: array(htmlAttribute)=?, ~children: React.element,
           ~key: string=?, unit) =>
           {. "children": React.element, "defaultTitle": string,
             "htmlAttributes": array(htmlAttribute),
             "meta": array(metaField), "title": string}
           = ""
       is not included in
         [@bs...] external makeProps:
           (~title: string=?, ~defaultTitle: string=?,
           ~meta: array(metaField)=?,
           ~htmlAttributes: array(htmlAttribute)=?, ~children: React.element,
           ~key: string=?, unit) =>
           {. "children": React.element, "defaultTitle": option(string),
             "htmlAttributes": option(array(htmlAttribute)),
             "meta": option(array(metaField)), "title": option(string)}
           = ""
       File "/.../src/H.re", line 10, characters 2-17: Actual declaration

rickyvetter · 2019-04-19T13:28:35Z

Looking at this.

rickyvetter · 2019-04-19T14:45:20Z

Hey @diligiant it looks like everything works! These types aren't correct.

  [@react.component]
  let make =
      (
        ~title: option(string)=?,
        ~defaultTitle: option(string)=?,
        ~meta: option(array(metaField))=?,
        ~htmlAttributes: option(array(htmlAttribute))=?,
        ~children: React.element,
      )
      : React.element => {
    <div />;

diligiant · 2019-04-19T16:26:19Z

@rickyvetter you are right, I made a mistake (my classic mistake) when I added the types per bob's request. BUT if I remove them (in the .re), the error comes back

Is this normal ?

The implementation src/H.mlast
       does not match the interface src/h-WaykeHelmet.cmi:
       Values do not match:
         [@bs...] external makeProps:
           (~title: 'title=?, ~defaultTitle: 'defaultTitle=?, ~meta: 'meta=?,
           ~htmlAttributes: 'htmlAttributes=?, ~children: React.element,
           ~key: string=?, unit) =>
           {. "children": React.element,
             "defaultTitle": option('defaultTitle),
             "htmlAttributes": option('htmlAttributes),
             "meta": option('meta), "title": option('title)}
           = ""
       is not included in
         [@bs...] external makeProps:
           (~title: string=?, ~defaultTitle: string=?,
           ~meta: array(metaField)=?,
           ~htmlAttributes: array(htmlAttribute)=?, ~children: React.element,
           ~key: string=?, unit) =>
           {. "children": React.element, "defaultTitle": option(string),
             "htmlAttributes": option(array(htmlAttribute)),
             "meta": option(array(metaField)), "title": option(string)}
           = ""
       File "/.../src/H.re", line 10, characters 2-17: Actual declaration

rickyvetter · 2019-04-25T04:19:06Z

I've created a slightly simpler case for this - in an re file you can add the following:

module Foo: {
  [@react.component]
  let make:
      (
        ~htmlAttributes: array(float)=?,
      )
      => React.element;
} = {
[@react.component]
let make =
    (
      ~htmlAttributes as _=?,
    )
    : React.element =>
  <div />;
};

Specific ingredients needed: optional argument with a complex type - array(foo) will break but foo won't.

 Values do not match:
    [@bs...] external makeProps:
      (~htmlAttributes: 'htmlAttributes=?, ~key: string=?, unit) =>
      {. "htmlAttributes": option('htmlAttributes)} = ""
  is not included in
    [@bs...] external makeProps:
      (~htmlAttributes: array(float)=?, ~key: string=?, unit) =>
      {. "htmlAttributes": option(array(float))} = ""

In this scenario you can see that the types do indeed match - which leads me to believe it has something to do with the hidden optional types. Investigating further. Sorry for the slow turnaround here!

diligiant · 2019-04-25T13:37:20Z

@rickyvetter don’t worry ; all that matters if that you find so that others don’t face this.

diligiant · 2019-04-26T09:42:35Z

@rickyvetter are you sure it's fixed?
I wanted to be really sure (after a simple git pull and rebuild) -> git clone buckescript, node scripts/buildocaml.js & install.js

yarn bsb -clean-world, -make-world
yarn run v1.15.2
$ /.../js-mono/node_modules/.bin/bsb -make-world

[.bin/bsb -> new repository - one can never be too careful]

Your simpler case still errs as well as mine (I checked that the changes you committed where there as well.)

rickyvetter · 2019-04-26T14:12:03Z

module Foo :
  sig
    external makeProps : ?bar:string array -> unit -> string = ""[@@bs.obj ]
  end =
  struct external makeProps : ?bar:'bar -> unit -> string = ""[@@bs.obj ] end

@bobzhang I think this is actually an issue with bs.obj. I'm not sure why it took so long to figure out.

bobzhang · 2019-04-26T15:45:59Z

looking into it, thanks for the small example

yawaramin · 2019-04-26T23:01:09Z

Aliasing the string array type works:

module Foo : sig
  type t
  type bar = string array
  external makeProps : ?bar:bar -> unit -> t = "" [@@bs.obj]
end = struct
  type t
  type bar = string array
  external makeProps : ?bar:'bar -> unit -> t = "" [@@bs.obj]
end

fix #3519

bobzhang · 2019-04-28T07:34:58Z

hi all, it should be fixed on master, let me know if it works, sorry for the delay

diligiant · 2019-04-28T08:22:17Z

@bobzhang I checked previous examples and it's fixed here. Thank you. I was able to uncomment other .rei (ones with default values for ex), so thank you again.

diligiant · 2019-07-02T15:06:33Z

@bobzhang the problem is "back/still" in 6.0.3 (5.0.5 is fine). You can reproduce with @rickyvetter minimal test case

We've found a bug for you!
  /…/rick.re 8:5-16:1

   6 │       )
   7 │       => React.element;
   8 │ } = {
   9 │ [@react.component]
   . │ ...
  15 │   <div />;
  16 │ };

  Signature mismatch:
  ...
  Values do not match:
    [@bs...] external makeProps:
      (~htmlAttributes: 'htmlAttributes=?, ~key: string=?, unit) =>
      {. "htmlAttributes": option('htmlAttributes)} = ""
  is not included in
    [@bs...] external makeProps:
      (~htmlAttributes: array(float)=?, ~key: string=?, unit) =>
      {. "htmlAttributes": array(float)} = ""
  File "/…/rick.re", line 2, characters 2-112:
    Expected declaration
  File "/…/rick.re", line 9, characters 2-17:
    Actual declaration

bobzhang · 2019-07-03T11:52:02Z

This looks weird, when I fix the bug, I added a regression test: https://github.com/BuckleScript/bucklescript/blob/master/jscomp/test/gpr_3519_test.ml
it works for 4.06.1

bobzhang · 2019-07-03T12:10:05Z

@rickyvetter could this be a bug in react-jsx 4.06?
you can see htmlAttributes has different type one is option, the other is array
this is different bug which I fixed last time (different marshalled string)

bobzhang · 2019-07-03T12:17:35Z

@diligiant if this is a new bug, would you mind open a new issue?

diligiant · 2019-07-03T12:57:11Z

@bobzhang thank you. I'll let @rickyvetter "answer" your previous hint and take it from here.

rickyvetter mentioned this issue Apr 26, 2019

use predef option for interface externals #3531

Merged

bobzhang closed this as completed in #3531 Apr 26, 2019

rickyvetter mentioned this issue Apr 26, 2019

Revert "use predef option for interface externals" #3533

Merged

bobzhang reopened this Apr 26, 2019

bobzhang added the priority:high label Apr 26, 2019

bobzhang added a commit that referenced this issue Apr 28, 2019

tentative fix #3519

bec802d

bobzhang closed this as completed in 81fb9e4 Apr 28, 2019

bobzhang added a commit that referenced this issue Apr 28, 2019

Merge pull request #3535 from BuckleScript/gpr_3519

b55e6f2

fix #3519

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new [@react.component] vs .rei #3519

new [@react.component] vs .rei #3519

diligiant commented Apr 18, 2019

bobzhang commented Apr 18, 2019 •

edited

Loading

diligiant commented Apr 18, 2019 •

edited

Loading

rickyvetter commented Apr 19, 2019

rickyvetter commented Apr 19, 2019

diligiant commented Apr 19, 2019

rickyvetter commented Apr 25, 2019 •

edited

Loading

diligiant commented Apr 25, 2019

diligiant commented Apr 26, 2019 •

edited

Loading

rickyvetter commented Apr 26, 2019 •

edited

Loading

bobzhang commented Apr 26, 2019

yawaramin commented Apr 26, 2019

bobzhang commented Apr 28, 2019

diligiant commented Apr 28, 2019

diligiant commented Jul 2, 2019

bobzhang commented Jul 3, 2019

bobzhang commented Jul 3, 2019

bobzhang commented Jul 3, 2019

diligiant commented Jul 3, 2019

new [@react.component] vs .rei #3519

new [@react.component] vs .rei #3519

Comments

diligiant commented Apr 18, 2019

bobzhang commented Apr 18, 2019 • edited Loading

diligiant commented Apr 18, 2019 • edited Loading

rickyvetter commented Apr 19, 2019

rickyvetter commented Apr 19, 2019

diligiant commented Apr 19, 2019

rickyvetter commented Apr 25, 2019 • edited Loading

diligiant commented Apr 25, 2019

diligiant commented Apr 26, 2019 • edited Loading

rickyvetter commented Apr 26, 2019 • edited Loading

bobzhang commented Apr 26, 2019

yawaramin commented Apr 26, 2019

bobzhang commented Apr 28, 2019

diligiant commented Apr 28, 2019

diligiant commented Jul 2, 2019

bobzhang commented Jul 3, 2019

bobzhang commented Jul 3, 2019

bobzhang commented Jul 3, 2019

diligiant commented Jul 3, 2019

bobzhang commented Apr 18, 2019 •

edited

Loading

diligiant commented Apr 18, 2019 •

edited

Loading

rickyvetter commented Apr 25, 2019 •

edited

Loading

diligiant commented Apr 26, 2019 •

edited

Loading

rickyvetter commented Apr 26, 2019 •

edited

Loading