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

<py-button> and <py-inputbox> should be killed, or at least they should not execute code #854

Closed
antocuni opened this issue Oct 14, 2022 · 9 comments

Comments

@antocuni
Copy link
Contributor

This is a proposal which would be a big change in how you use <py-button> and <py-inputbox> but I think that the current status causes too many problems and it should be fixed.

Currently, there are three tags which causes the execution of python code: <py-script>, <py-button> and <py-inputbox>. There is also <py-repl> but it is a slightly different case because it executes code only upon user interaction, so we can ignore it for now. In the following I will only talk about <py-button> but the same applies to <py-inputbox> of course.

The main reason for existence of <py-button> is to be able to easily define an on_click event:

            <py-button label="my button">
                import js
                def on_click(evt):
                    js.console.log('clicked!')
            </py-button>

But this it very easy to do the same using already existing features, e.g.:

            <py-script>
                import js
                def on_click():
                    js.console.log('clicked!')
            </py-script>
            <py-button label="my button" py-click="on_click()"></py-button>

Moreover, there are ongoing discussions to make python event handlers even easier/better, see e.g.: #835

Moreover, the current implementation of <py-button> is a complete hack: basically, we search for a literal def on_click string in the source code and substitute it with def on_click_SOMETHING, where SOMETHING is the id of the element.

this.code = this.code.split('self').join(this.mount_name);
let registrationCode = `from pyodide.ffi import create_proxy`;
registrationCode += `\n${this.mount_name} = Element("${mainDiv.id}")`;
if (this.code.includes('def on_focus')) {
this.code = this.code.replace('def on_focus', `def on_focus_${this.mount_name}`);
registrationCode += `\n${this.mount_name}.element.addEventListener('focus', create_proxy(on_focus_${this.mount_name}))`;
}
if (this.code.includes('def on_click')) {
this.code = this.code.replace('def on_click', `def on_click_${this.mount_name}`);
registrationCode += `\n${this.mount_name}.element.addEventListener('click', create_proxy(on_click_${this.mount_name}))`;
}
// now that we appended and the element is attached, lets connect with the event handlers
// defined for this widget
await runtime.runButDontRaise(this.code);
await runtime.runButDontRaise(registrationCode);
logger.debug('py-button connected');

this is problematic in many ways and it doesn't really scale if we want to support more events.

The other big problem is the order of execution of tags, see #761.
Currently, the various <py-*> tags are executed in an order which doesn't match the one in which they appear in the DOM: as soon as we do customElements.define('py-script', ...) we execute all <py-script> tags. The we define <py-button> and we execute all <py-button> tags, etc. etc.
There are at three two ways of solving the problem:

  1. define all the custom elements before the DOM is loaded. This might work but it doesn't play well e.g. with plugins (which could define more elements later)
  2. the various connectedCallback should put code in a queue, which is then ordered in a way to match the DOM order. Writing this sorting function will be complex, error prone and probably inefficient though. Moreover, we would also need a way to periodically execute scripts in the queue if they are added dynamically to the DOM
  3. don't use connectedCallback: instead, at some point during the startup we walk the DOM, collect all the <py-script>, <py-button>, <py-inputbox> etc. and execute their code in the right order. This would work, but then we loose the possibility of dynamically add a <py-script> element to the DOM.

I think that the current behavior of <py-button> & co. causes too many problems and it's not worth of. I propose to kill it and rely on the standard py-click attributes to define event handlers.

Then at some point which should also discuss whether we want to have <py-button> & co. as builtin tags of PyScript: I think that the original idea was to provide some sort of "easy UI toolkit", and I think we should totally provide one, but probably as a "standard plugin" which can be disabled, because I'm sure that very soon the community will start to develop their own frameworks on top of pyscript. But this is probably a discussion for another issue.

/cc @fpliger because he's the original author and @tedpatrick because I'm sure he has opinions.

@tedpatrick
Copy link
Contributor

tedpatrick commented Oct 14, 2022

I feel these components should not exist in PyScript. I view them as signals that the event API is not simple enough. Rather than reinvent button, input, and (long list of other elements), we should integrate more cleanly with existing elements via attributes.

<button py-click="foo()">Click Me</button>

<input py-input="upload(event)" type="file">

<select py-input="selected(event)">
  <option>Volvo</option>
  <option>Saab</option>
  <option>Mercedes</option>
  <option>Audi</option>
</select>

<canvas py-mousemove="draw_stuff(event)"></canvas>

<py-script>
import js

@handler( event="load")
def when_it_all_loads(e):
    js.console.log("It all loaded") 

@handler
def foo():
    js.console.log("foo called")

@handler
def upload(e):
    js.console.log("upload called")

@handler
def selected(e):
    js.console.log("selected called")

@handler
def draw_stuff(e):
    js.console.log("draw_stuff called")
</py-script>   

@tedpatrick
Copy link
Contributor

Assuming our event implementation supported all event types... we can add custom lifecycle events for PyScript.

<div style="display:none" py-start="show(event)">
My app elements
</div>

<py-script>
import js

@handler( event="py-start" )
def start_me(e):
    js.console.log("pyscript started") 

@handler( event="py-load" )
def load_me(e):
    js.console.log("pyscript loaded")

@handler
def show(e):
    e.target.style = ""
</py-script>

@antocuni
Copy link
Contributor Author

I feel these components should not exist in PyScript.

I feel the same but I was not bold enough to propose their complete removal :).

. I view them as signals that the event API is not simple enough. Rather than reinvent button, input, and (long list of other elements), we should integrate more cleanly with existing elements via attributes.

100% agree. I changed the title of this issue to reflect the possibility of completely killing them

@antocuni antocuni changed the title <py-button> and <py-inputbox> should not execute code <py-button> and <py-inputbox> should be killed, or at least they should not execute code Oct 15, 2022
@pauleveritt
Copy link

I wonder if there's room for some prototypes outside the current codebase. Kind of R&D efforts.

I feel like the mentions here of custom events and mutation observers are really on the right track, to kind of a state model with dispatchers that can be plugged in using normal platform stuff.

@tedpatrick
Copy link
Contributor

tedpatrick commented Oct 15, 2022 via email

@tedpatrick
Copy link
Contributor

tedpatrick commented Oct 15, 2022

  1. Added the https://github.com/pyscript/research
  2. Added WIP event research as a proposal here

@antocuni
Copy link
Contributor Author

I wonder if there's room for some prototypes outside the current codebase. Kind of R&D efforts.


Part of the effort to simplify things is also aligned with defining a lifecycle that supports plugins. Plug-ins can range from new components to alternative frameworks above PyScript as a whole.

yes exactly, this was one of the main reasons behind my plugins ideas. I would like PyScript to have a very small, strong, cohesive and powerful core: once we have that, we can start building higher-level APIs and building blocks on top of the core, in form of plugins.
Some of the plugins will be developed by us and be part of some sort of "pyscript stdlib", but nothing will stop us and the broader community to develop and experiment with other ideas.
15 years ago there was a time in which every week a new Python web framework was announced (and in the JS world I think it still happens today :)): we should make sure that this kind of experimentation is possible also with PyScript.

@JeffersGlass
Copy link
Member

These two custom elements were deprecated by #931 and removed by #1084.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

4 participants