-
-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: produce a hard error for bs.*
attributes
#1034
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I think this will be very useful to see what deps fail to build when publishing v3 in opam.
I noticed that extensions like mel.raw
or mel.re
aren't handled in the PR, do they go through a different path that doesn't have this problem?
ppx/ast_attributes.ml
Outdated
let warn_if_non_namespaced ~loc txt = | ||
if not (Mel_ast_invariant.is_mel_attribute txt) then | ||
Mel_ast_invariant.warn ~loc Deprecated_non_namespaced_attribute | ||
let warn_if_bs_or_non_namespaced ~loc txt = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the function could be renamed to error_if_bs_or_non_namespaced
.
ppx/ast_attributes.ml
Outdated
@@ -130,7 +140,9 @@ let process_attributes_rev attrs : attr_kind * attribute list = | |||
let process_pexp_fun_attributes_rev attrs = | |||
List.fold_left | |||
~f:(fun (st, acc) ({ attr_name = { txt; _ }; _ } as attr) -> | |||
match txt with "mel.open" -> (true, acc) | _ -> (st, attr :: acc)) | |||
match txt with | |||
| "mel.open" | "bs.open" -> (true, acc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this attribute do? It's not documented in melange.re.
Also, should warn_if_bs_or_non_namespaced
be called in this case too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mel.open
extracts an OCaml exn
from an external JS data source. Check some old docs here: https://melange.re/melange/Manual.html#__code_bs_open_code_type_safe_external_data_source_handling_since_1_7_0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* feat: produce a hard error for `bs.*` attributes * chore: address code review items
bs.*
attributes means that some modules now produce errors at runtime if they have old code -- this is unacceptable.bs.*
attribute