reactive-python / reactpy-django

It's React, but in Python. Now with Django integration.
https://reactive-python.github.io/reactpy-django/
MIT License
323 stars 18 forks source link

View to Component #81

Closed Archmonger closed 1 year ago

Archmonger commented 2 years ago

Changelog

IMPORTANT NOTE

This PR does not add automatic handling of things such as href click events or form submission. Currently compatibility=True is required to get href and forms to work.

This is a desirable behavior that should be implemented in a future PR as a toggleable option.

In the future view_to_component should be able to automatically perform any URL routing/re-rendering that is needed. The VDOM component should essentially "act as if it's an iframe", despite it being VDOM.

Remaining Todo:

Archmonger commented 2 years ago

Blocked on:

Archmonger commented 2 years ago

@rmorshea I'm having some issues with @idom.component type hints.

Between these two files

Seems like immediately returning a component within a component causes type hint issues. Additionally, embedding a view_to_component within a idom.html.div can cause type hint issues as well.

Archmonger commented 2 years ago

@rmorshea Just merged some mypy warning fixes, but that's really a kick in the teeth for my pylance dev environment.

mypy does not like type: ignore in situations where it doesn't believe it needs it. And pylance has warnings on several more LOCs than mypy.

Can we consider using pyright (the type checker built into pylance) instead of mypy?

Otherwise, I'm just going to have to learn to live with seeing type errors all over our code.


EDIT: I'll dig into whether I can disable Pylance type checking and enable mypy within vscode

EDIT 2: Looks like I can, I'll make sure to make this a default within our future vscode configuration file.

Archmonger commented 2 years ago

@rmorshea Just merged with main, so this PR is ready for review. There's some features I'd like to add to this in the future, but I don't believe that should be addressed in this PR.

I will make a new Django-IDOM release as soon as this gets merged in.

Archmonger commented 1 year ago

@rmorshea I'm realizing a limitation with IDOM Core's view_to_component.

Currently, because event handlers can't be JavaScript strings in IDOM core, inline events cannot be serialized into VDOM.

For example, onclick events like this won't work: <button onclick='example()'></button>

rmorshea commented 1 year ago

@Archmonger, that's actually more of an issue with React. If you assign onclick="..." IDOM will pass that on to the client as-is. However React does not interpret strings as event handlers. I think the only way to do this would be to dangerouslySetInnerHtml at which point it doesn't seem like you'd need html_to_vdom at all since you could literally use the raw HTML string.

Archmonger commented 1 year ago

I would say this is ends up being a pretty big limitation. When working within JavaScript frameworks, this issue doesn't really exist.

I can't think of a safe way of fixing this.

cough Even our existing script tag implementation is potentially unsafe.

Might want to document the limitation in core, probably within the html_to_vdom docstring.

Archmonger commented 1 year ago

On a different note, I think the revised tests for script, request, args, and kwargs are solid.