feat(ppx): separate @@deriving abstract into jsProperties, getSet
#987
Conversation
…ers_setters`
ppx/melange_ppx.ml
Outdated
| Deriving.Generator.V2.make (args ()) (fun ~ctxt:_ (rf, tdcls) -> | ||
| Ast_derive_abstract.handleCstrTdclsInSig rf tdcls) | ||
| in | ||
| Deriving.add ~str_type_decl ~sig_type_decl "make_opt_keys" |
There was a problem hiding this comment.
is "make" necessary, or could it be just opt_keys? or opt_js_keys as you mentioned including "js" somewhere.
There was a problem hiding this comment.
yeah, I'm open to changing the name; I'd prefer keeping something that mentions it makes a JS object, e.g. jsConstructor or jsObject
There was a problem hiding this comment.
I was thinking about the JS hasOwnProperty method. It seems a property is the mix of a key and a value, so maybe the deriver name should include it.
It'd be nice to define it like "constructor that skip properties in the JS object if the field is None", maybe optJsProperties, makeOptProperties? or skipPropertiesConstructor?
There was a problem hiding this comment.
I'd prefer keeping something that mentions it makes a JS object,
I am not sure this is very informative. All records compile now to JS objects so a name related to that wouldn't be very meaningful imo.
There was a problem hiding this comment.
ok, I called it jsProperties to unblock the PR, but we can brainstorm a better name later on.
oh absolutely. I'll change that before merging for sure |
|
9d55611 implements Will deprecate it in a subsequent PR. |
anmonteiro
force-pushed
the
anmonteiro/split-deriving-abstract
branch
from
4640961
to
4afbf67
Compare
anmonteiro
changed the title
@@deriving abstract into make_opt_keys, getters_setters@@deriving abstract into jsProperties, getSet
No description provided.