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

report better error for attrs that cannot be serialized #1008

Merged
merged 13 commits into from
Jun 15, 2023

Conversation

rmorshea
Copy link
Collaborator

@rmorshea rmorshea commented Jun 6, 2023

Issues

closes: #930
closes: #437

Summary

In reactpy.core.vdom.vdom check if attributes are JSON serializable when in debug mode.

Also misc changes to config options to make this easier in particular ability to specify options as parents.

Checklist

  • Tests have been included for all bug fixes or added functionality.
  • The changelog.rst has been updated with any significant changes.

@Archmonger
Copy link
Contributor

Archmonger commented Jun 7, 2023

We might also want to validate event handlers too. Need to ensure they're either coroutines or callable.

@rmorshea rmorshea marked this pull request as ready for review June 7, 2023 19:53
@rmorshea
Copy link
Collaborator Author

rmorshea commented Jun 7, 2023

We might also want to validate event handlers too.

Event handlers are already extracted from attributes based on whether they are callable or not.

@rmorshea rmorshea force-pushed the better-json-ser-error branch 8 times, most recently from e20da5c to 73296af Compare June 15, 2023 03:39
@rmorshea rmorshea merged commit a3d79aa into reactive-python:main Jun 15, 2023
17 checks passed
@Archmonger
Copy link
Contributor

Archmonger commented Jun 15, 2023

We might also want to validate event handlers too.

Event handlers are already extracted from attributes based on whether they are callable or not.

I currently don't see any warnings if I put in a string as an event handler.

from reactpy import component, html, run


@component
def test():
    return html.div({"on_click": "do nothing"}, "Hello World")


run(test)

@Archmonger
Copy link
Contributor

Also I don't see how this PR resolves #437

@rmorshea
Copy link
Collaborator Author

I currently don't see any warnings if I put in a string as an event handler.

This is strictly about ensuring that attributes are JSON serializable. Know what values are allowed for each attribute on each HTML element would need to be done in a follow-up PR.

@rmorshea
Copy link
Collaborator Author

Also I don't see how this PR resolves #437

In debug mode, users will get an error if they attempt to pass non-JSON-serializable props to a JS components. They'd get that error without debug mode too, but in debug mode you get a more helpful one.

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.

Better Traceback for JSON Serialization Failures Explain that component attributes become uninterpreted JSON
3 participants