Skip to content

Conversation

ELLIOTTCABLE
Copy link
Contributor

@ELLIOTTCABLE ELLIOTTCABLE commented Jan 15, 2021

At the moment, @@bs.deriving (and other BuckleScript-interpreted attributes) cause a fatal error if they encounter payloads they don't understand in the AST. This is problematic as of #4701, now that BuckleScript's @@deriving and OCaml-native @@deriving (from ppx_deriving) attempt to interpret the same key:

type foo = {
  a: int;
  b: int;
}
[@@deriving jsConverter]

type bar = {
  x: int;
  y: int;
}
[@@deriving yojson { strict = true }]
File "test_deriving.ml", line 11, characters 3-11:
Error: this is not a valid record config

For compatibility's sake, I've reduced this error to a compile-time warning (w+104, the same quieting-category as used in the other 'idk wtf to do with this' deriving-case).

After these changes, the above snippet will yield this warning:

File "test_deriving.ml", line 11, characters 3-11:
Warning 104: bs.deriving warning: invalid attribute config-record, ignoring

Ideally, I'd like to see this more fully understand OCaml-native ppx_deriving payloads - not to interpret them, but to provide better error-messages (i.e. differentiate between "uninterpreted ppx_deriving payload" and "genuinely typo'd-or-incorrect payload"), but for now, at least this fix unblocks existing hybrid BS/native projects, and will allow me to fix the bs-deriving compatibility project!

At the moment, @@bs.deriving (and other BuckleScript-interpreted attributes) cause a fatal error if they encounter payloads they don't understand in the AST. This is problematic now that BuckleScript's @@deriving *and* OCaml-native @@deriving (from ppx_deriving) attempt to interpret the same key:

```ocaml
type foo = {
  a: int;
  b: int;
}
[@@deriving jsConverter]

type bar = {
  x: int;
  y: int;
}
[@@deriving yojson { strict = true }]
```

For compatibility's sake, I've reduced this error to a compile-time *warning* (w+104, the same quieting-category as used in the other 'idk wtf to do with this' deriving-case).

Ideally, I'd like to see this more fully understand OCaml-native ppx_deriving payloads - not to interpret them, but to provide better error-messages (i.e. differentiate between "uninterpreted ppx_deriving payload" and "genuinely typo'd-or-incorrect payload"), but for now, at least this fix unblocks existing hybrid BS/native projects, and will allow me to fix the bs-deriving compatibility project!
@bobzhang
Copy link
Member

looks good, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants