Skip to content
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

Remove raise annotations and fix locations on errors #863

Merged
merged 14 commits into from
Nov 25, 2024

Conversation

davesnx
Copy link
Member

@davesnx davesnx commented Nov 20, 2024

Depends on #862 that depends on #859, that depends on #846

Minimalist and spiritual successor of #848

Fix #843

Took the chance to cleanup all raise Invalid_argument into Location.raise_errorf (which isn't ideal from the ppxlib standpoint, since it fails a Parseetree generation and only shows one error) but the refactor to this is massive

@davesnx davesnx changed the title Add test for error on key Remove raise annotations and fix locations on errors Nov 20, 2024
Copy link
Collaborator

@jchavarri jchavarri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. But why does it depend on all those other PRs? Seems that it could be implemented based on main directly?

Base automatically changed from ref-as-valid-prop to 19 November 25, 2024 18:01
@davesnx davesnx changed the base branch from 19 to main November 25, 2024 18:09
@davesnx
Copy link
Member Author

davesnx commented Nov 25, 2024

Made it depend on #main @jchavarri

Deleted the test that ensures ref works, since it come in #846 (and depends on ReactTestingLibrary dependency)

@davesnx davesnx merged commit f83f216 into main Nov 25, 2024
3 checks passed
@davesnx davesnx deleted the remove-raises-as-annotations branch November 25, 2024 18:27
davesnx added a commit that referenced this pull request Nov 25, 2024
* 'main' of github.com:/reasonml/reason-react:
  Remove raise annotations and fix locations on errors (#863)
  ppx: support "custom children" in uppercase components without having to wrap in array literal (#823)
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.

Very misleading error message if you misuse [@react.component]
2 participants