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

Completely separate this from rendering context #1317

Open
sdegueldre opened this issue Jan 11, 2023 · 4 comments
Open

Completely separate this from rendering context #1317

sdegueldre opened this issue Jan 11, 2023 · 4 comments
Labels
owl-3 features for owl 3
Milestone

Comments

@sdegueldre
Copy link
Contributor

sdegueldre commented Jan 11, 2023

Currently, in some cases, calling component methods without qualifying them with this (eg myMethod() instead of this.myMethod()) inside the template of a component can cause subtle bugs: because a bare function call is transpiled by owl into a method call on the rendering context (ctx['myMethod']()), that function will be bound to the rendering context, which can cause several issues. In particular, any "write" in the function will be written on the rendering context instead of the component, and if a variable in the rendering context shadows a property of the component, the method will read that instead.

This is particularly confusing because for the most part, the method will behave as expected in most respects, and so a method call can be written incorrectly and still work, until a code change adds a write to the method which appears to mysteriously not work, the fix needing to be applied being in an entirely different place from the original code change.

The issue is further aggravated by the existence of getters, which makes any unqualified property read unsafe. The issue can be further obscured if for some reason the method or getter returns a handler, as that handler will be bound to the wrong this but the bug will only appear when that handler is called at a later point, and again only on the condition that it attempts to write on a component or reads a variable that is shadowed by the rendering context.

@sdegueldre sdegueldre added the owl-3 features for owl 3 label Jan 11, 2023
@sdegueldre sdegueldre added this to the Version 3.0 milestone Jan 11, 2023
@aab-odoo
Copy link
Contributor

aab-odoo commented Jan 11, 2023

also, should it be named this or comp or whatever (in qweb, it was widget)

@sdegueldre
Copy link
Contributor Author

sdegueldre commented Jan 12, 2023

Alternative: reimplement t-set/t-foreach so that they don't need to protect the context, ie the rendering context is only the scope of the rendering function and during compilation don't look up template-local variables in the context.

@ged-odoo
Copy link
Contributor

@sdegueldre I think that it was done that way in owl 1.0, or in some early prototype. The problem is how can you tell in a t-called template that a variable is local to the template?

<t t-name="mycomponent">
  <t t-set="a" t-value="1"/>
 <t t-call="subtemplate"/>
</t>
<t t-name="subtemplate">
  <t t-esc="a"/> <!-- local -->
  <t t-esc="b"/> <!-- not local, should look in component-->
</t>

Maybe we can change the expression "compiler" to do a lookup in a scope object, and fallback on ctx object. So, instead of transforming a into ctx['a'], we could transform it in: ('a' in scope ? scope['a'] : ctx['a']).
It's annoying to have such a complex expression in most cases :(

@ged-odoo ged-odoo mentioned this issue Jul 26, 2023
@ged-odoo
Copy link
Contributor

Note that having a clear and obvious distinction (statically known at compile time) between "local" and "component" variables should allow Owl to simplify the code handling scopes. Currently, it has to capture the current scope in many cases, and it has to perform various kind of weird shenanigans when writing/updating values. Doing what this issue propose would remove all that, which would also slightly accelerate owl rendering speed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
owl-3 features for owl 3
Projects
None yet
Development

No branches or pull requests

3 participants