When fixing the absence of capture for arrow functions passed as props,
we unintentionally introduced a breaking change: bare function calls in
the arrow functions used to be called with the rendering context as
their this value and this was no longer the case.
This commit fixes that by intentionally not capturing the value of
functions that are called withing the arrow function.
In some very rare cases (such as the use of the t-foreach directive),
Owl did leak the values in the render context in the global context.
This was due to the fact that the compiled template looked like this:
let _3 = _4 = _5;
instead of
let _3 = _5;
let _4 = _5;
If we want to upgrade Typescript to version >4 (in this repo or another using
owl), we also need jest >25. Jest <26 do not support Typescript >4.
However, jest >26 has a few breaking changes that completely breaks owl in
tests.
This commit updates jest to the current latest version (27) and adapts the
code accordingly.
A few words on what changed
---------------------------
1. the default test environment is no longer `jsdom`. It is now manually
configured to restore the previous env.
2. the jsdom version has been upgraded. This brings a few breaking changes,
detailed later.
3. there's a bug in jsdom >16.4. Manually created `<t/>` (by qweb compilation)
are recognized as "T" and not "t". Qweb thinks it's a component (since the
first letter is capitalized), but it's not a component: boom, everything
breaks.
A fix has been proposed here jsdom/jsdom#3240. But it's not likely to land
in jest in the short term (jest would need to update its dependency to the
next major jsdom version). This commit works around the problem for now by
creating `<t/>` slighty differently such that they are in an XML document
from the start (and not HTML document).
4. the xml parser implementation changed to increase the strictness and
correctness of XML parsing, according to specifications. A few tests needed
to be adapted.
https://github.com/jsdom/jsdom/commit/c96decf837ece54bdc550dfb7dca7e5d6c97bc2d
5. jest matcher `toHaveProperty` now check inherited properties. This breaks a
few tests. Since the breaking assertions didn't bring a lot of value from a
behavior point of view (it was more "white box" technical tests), they are
removed in this commit
https://github.com/facebook/jest/commit/1256f76a5a83034b51c7524142b60b099f69a7ab
6. jsdom now implements the behavior of links (`<a/>`). One test was `click`ing
on such a link, with the right click. Jsdom now tries to navigate to the
pointed URl...but crashes because navigation is not implemented :(
It turns out the test is probably not a valid/realistic scenario. On every
tested browser (chrome - chromium - brave - edge -firefox - safari), a
right click with the mouse triggers a `contextmenu` event and no `click`
event.
https://github.com/jsdom/jsdom/commit/cc95abc576f596ff7f3eaf8245f376e1f21aa485
Previously, if two attributes in t-att-class shared some classes, their
presence would be determined by the last attribute declared, instead of
being present if any attribute containing it evaluates to true. This
commit fixes that.
The previous changes in the combine method (used to copy all the
variables defined in the current scope for use in a slot) had the effect
of squashing the prototype chain: instead of `Component -> Obj1 ->
Obj2 -> Obj3`, the combined scope had: `Component -> Obj1'`.
This has an unfortunate interaction with the way t-call is implemented,
which uses the fact that we are in a subscope to add a own
__access_mode__. It depends specifially on the prototype chain, and that
the parent scope may have a different value for that property. But with
the way combine was implemented, we lost all that subtlety since
everything is squashed.
In this commit, we reimplement that function in a way to make sure we
keep the prototype chain structure
Before this commit, Owl inline expressions with a list with multiple
elements such as [a,b,c] was transformed into
[scope['a'], b: scope['b'], scope['c']]
instead of
[scope['a'], scope['b'], scope['c']]
This is due to a previous commit adding support for short object
descriptions such as {a,b}.
To fix this means that we have to keep track of the current group type
for the expression, which is done by using a stack.
In some situations (a slot inside a slot), the combine method was
wrongly copying all properties of the scope into the context, which
caused the event handling system to wrongly use a subobject as component
(since it detects the fact that __owl__ is a own property_).
Consequently, we could have very subtle issue with some properties being
shadowed by a sub object.
There is a possibility that the transitionend event of an element/component
with t-transition directive won't trigger. Though this situation is
difficult to assert, it was observe in odoo runbot for the pos ui.
When the transitionend event is not fired, the callback that removes
the element from the dom won't be called, resulting to a corrupted view.
An example of which is the following:
```html
<div t-if="show" t-transition="fade">Hello</div>
```
If `show` is set to false by some ui action and by any chance the
transitionend event is not fired (perhaps because the transition didn't
actually start or because of completely unknown reason), the div element
will remain in the view -- and this is not desirable.
This commit patches this situation such that if after 50ms that the
transitionend event is supposed to be fired but the event isn't fired, we
force the callback using a setTimeout. This guarantees the call of the
callback that is suppose to remove the element from the view.
the low level method htmlelement.classList.add does not accept multiple
classes in one string, which is why, in owl, the expression
`<div t-att-class="{'a b c': value}" />`
did not work as one might expect. It is however very convenient in real
life templates, so this commit improve owl by adding support for this
feature.
closes#813
The templates contained in a slot have to be rendered with the current
scope, but during the rendering of the sub component, which happens
later (after the willStart for the sub component). Therefore, we need to
save the scope that should be used for the slots, so we can access the
proper variables. This was done using an Object.assign() statement,
which is actually only a shallowclone for the own properties.
In this commit, we properly copy all the properties for the current
scope, even those contained in the prototype.
closes#855
Before this commit, it was not possible to set the value of a select tag
by using the t-att-value attribute.
Doing so is not actually trivial because of the way the vdom works: it
processes the node attributes before its children are created, which
means that the vdom code tries to set the initial value of the select
before its children are created, which means that it is ignored. To make
it work, I added a node create hook which is called after the children
are completely processed.
closes#873
Before this commit, using an expression such as "{machin}" was compiled
in qweb into "{scope['machin']}" which is not valid.
With this commit, we instead transform it into "{machin:
scope['machin']}".
closes#885
Have a t-call within a t-set-slot of a component.
The called template has a t-component directive.
It should be like:
```xml
<t t-name="Zero">
<Slotted>
<t t-call="someTemplate" />
</Slotted>
</t>
<t t-name="someTemplate">
<SomeComponent />
</t>
```
Before this commit, the parent of SomeComponent was the Zero component
After this commit, the parent of SomeComponent is the Slotted Component as it should be
closes#862
The initial problem solved by this commit is that it was possible to get
into a situation where a mounting/rendering was started, then the component was
updated, but then another mounting operation begins, and it tries to
reuse the previous rendering operation, which is no longer uptodate.
The underlying issue is that Owl did not track properly the various
internal state change of a component. These issue should be solved by
the introduction of the status enum, which currently tracks 6 possible
states:
- CREATED
- WILLSTARTED
- RENDERED
- MOUNTED
- UNMOUNTED
- DESTROYED
This status number replaces the isMounted and isDestroyed boolean flags.
It has the advantage of making sure that the component is in a
consistent state (it is no longer possible to be destroyed and mounted,
for example)
Another advantage is that it gives us an easy way to track the fact that
a component has been rendered, but is not in the DOM. This is a subtle
situation where some various events can happen, and we need to be able
to react to that case.
Note that there is a change of behaviour: if a component is mounted in a
specific target, then before the mounting is complete, the component is
mounted in another target, we no longer reject the first mounting
operation.
The QWeb expression parser use an object as a mapping between some
strings and the desired output in the compiled template. However, as we
should all know, objects are not Maps, they have some additional
properties, such as "constructor" or "hasOwnProperty".
The simple solution is to make sure the mapping object does not have
anything in its prototype chain to pollute its purpose.
closes#835
From the beginning, Owl was designed to only call event handler on
components that are mounted in the DOM. The main benefit is that if a
component is destroyed, we are guaranteed to not execute any useless (or
potentially dangerous code).
However, there is one downside: if a component tree is being mounted,
and a child component trigger an event in its mounted hook, then it
cannot be caught by the parent, since the parent is technically not yet
mounted.
This may not be a good situation, but the point is that Owl
unnecessarily prevent the handler to be called.
We can fix this issue by simply checking if the component is not destroyed
instead of checking that it is mounted.
closes#809
It is useful to be able to disable the t-transition directive for
testing purposes. This commit introduces a global config flag to do
just that.
closes#768
Whenever a top level t-call was made with some non empty body, Owl
complained that a template should not have more than one root node.
The reason was that the compilation context for the body of the t-call
directive was the same as the root compilation context, and it already
had a parentNode set.
To fix the issue, this commit simply use the subContext method to create
a different compilation context, which actually makes sense, because the
body of a t-call is really a different situation. Also, as a bonus, it
slightly improves the code for the t-call directive.
closes#760
Strangely, the _compile method required a CompilationContext whenever it
was compiled as a sub template, but this parent context was actually not
really needed. I guess that it was the case in the past, but this was
changed at some point.
This commit makes another significant change: the xml element is no
longer mandatory. It is actually only required for slots (because the
template is not registered to qweb).
Finally, the interface for the whole method has been changed to use an
option object, which makes more sense with 3 optional paremeters.
Before this commit, the t-model directive worked well with expressions
such as "state.value", but not with bracketed expression: "state[value]"
(it generated invalid code).
This commit make the t-model smarter by detecting this case, and
properly capturing the base expression and key variable.
closes#694
Input with type="checkbox" have a special property (indeterminate) to
visually display the fact that the input value is non determinate (in my
chrome browser, the checkbox is then drawn with a simple - inside). It
does not actually modify the value of the input, only the way it is
displayed.
So, with this commit, owl will properly set the property, as expected.
closes#713
This is a rarely (if ever) used feature, but according to our qweb
reference implementation, it is possible to use the
t-call directive on an arbitrary html tag, like this:
<div t-call="my.template"/>
It is then interpreted as:
<div><t t-call="my.template"/></div>
So, with this commit, we make sure that the owl qweb implementation
matches that behaviour.
closes#706
Promises are kind of special, and do not behave like usual javascript
values.
For this issue, the problem is that when the observer tries to observe a
promise value, the code will crash with an error like this:
Uncaught TypeError: Method Promise.prototype.then called on incompatible receiver [object Object]
Also, note that it does not make much sense to proxify promise methods
anyway, since they are not (supposed) to be modified.
So, with this commit, we simply consider that promises should be treated
like a primitive value: simply ignored when determining if it should be
proxified
Note that I actually believe that putting promises in a useState is not
a good idea in general.
closes#677
The previous fix (overriding tostring of VDomArray) is actually more
general, and solves the same issue. So, let us simplify the code and
keep the more general solution.
This reverts commit 3bf91afc3f.
Consider this scenario:
- a variable v (with a body) is defined in a template
- it is then passed to a sub component as a prop
- and now, it is t-esc-ed.
Before this commit, the displayed value was [object object], because the
value actually passed to the sub component was a VDomArray (internal
structure used to represent nodelists)
This issue is actually quite a problem in practice, because values in a
templates are translated, but not in attributes. Therefore, using a
t-set directive with a body text content is the proper way to have
translated values at runtime.
We override in this commit the method toString of VDomArray to make sure
it is properly displayed.
Note that we considered changing the way props were generated (by trying
to detect VDomArray, then calling vDomToString), but then the value
would not be able to be used in a t-raw. Also, it is quite elegant to
be able to format the VDomArray only at the end.
closes#670
Sometimes, HTML is slightly more subtle than what I initially expect.
Rendering some html is simple, we have tags and attributes. However,
once we add behaviour, then the situation is more complex:
<input value="abc"/>
is an input with an INITIAL value of "abc", but the attribute does not
actually represent the CURRENT value of the input, which may be
different if the user did change it.
This is basically the difference between "attribute" and "property".
So, when rendering html with owl, we sometimes want to actually set
the property (current value), instead of the html attribute.
This commit make sure that this is the case for inputs with the "value"
attribute.
closes#722
The body of a t-call directive may be used to define private variables
to the sub template call.
However, the code that handles t-call worked like this:
- compile sub template if necessary
- then compile body of t-call to extract variables
This means that the variables defined in the t-call body were not yet
processed and available in the context. Because of that, when the call
to t-esc is done, there is not internal qweb var, and the code simply
outputs a scope['varname'], which is in our case a VDOMArray, so it is
displayed as [object object]
What this fix does is changing the way t-esc works: if we are in the
context of a sub template, then it assumes that any outside variable may
or may not be a VDomArray, so it needs to check and eventually convert
it to a string, if necessary.
closes#719
Owl has to manage a lot of interesting situations. One of them is when
a rendering is initiated, which creates a sub component, but then
another rendering starts, which invalidate the previous one, and will
create another sub component. Since the first sub component was not
ever in the DOM, we cannot rely on the vdom patching process to remove
it, so we have to do it manually.
Sadly, this is actually a very tricky situation, since there are other
subtle situations where the code that remove an unmounted widget could
be executed, in particular when the parent component is unmounted, then
remounted, then modified to trigger yet another rendering.
In this commit, we handle this case more carefully by making sure that
the destroyed subcomponent properly configures its pvnode so the patch
process happens as expected.
joint work with the framework team, and in particular LPE for his work on
finding a testcase!
closes#724, #731
Have a t-call nested in a t-foreach nested in a t-foreach
```xml
<t t-name="template">
<t t-foreach="..." t-as="a">
<t t-foreach="..." t-as="b">
<t-call="templateCalled" />
</t>
</t>
</t>
```
Before this commit, the `a` variable was not accessible within the t-call.
That was because the way t-call protected its scope by hiding other protected scope
in this case, the first protected scope for the first `t-foreach` was hidden
After this commit, `a` and `b` are accessible in the t-call, whether the t-call
defines its own variables by `t-set` or not.
Also, as expected from other fixes, there is no leaks of variables defined within a `t-call`
fixes#695
With QWeb, we can register globally templates (using the xml tag or
the registerTemplate function). However, these templates, once
compiled, can generate sub template compiled functions. Before this
commit, these sub functions were local to a specific instance.
This means that creating a new QWeb instance and rendering a global
parent template would crash, since it was unable to find the actual sub
function.
This commit fixes the issue: the sub functions are now shared
statically, but with a unique ID, so we do not have issues with sub
functions having a same name in different QWeb instance.
closes#701
Before this commit, the generated code was incorrect, and crashed,
when there was a t-esc="abc" in a subtemplate, with t-set="abc"
done outside the subtemplate, with syntax <t t-set="abc">value</t>.
There was some code in qweb to make sure that we support setting class
and t-att-class on the same html element:
<div class="some class" t-att-class="{b: true}">...</div>
But the code did not work in the other direction:
<div t-att-class="{b: true}" class="some class">...</div>
With this commit, we just add the missing if statement
closes#664
Have something like
```xml
<div t-name="Parent">
<t t-call="nodeTemplate">
<t t-set="recursive_idx" t-value="1"/>
<t t-set="node" t-value="root"/>
</t>
</div>
<div t-name="nodeTemplate">
<t t-set="recursive_idx" t-value="recursive_idx + 1"/>
<p><t t-esc="node.val"/> <t t-esc="recursive_idx"/></p>
<t t-foreach="node.children or []" t-as="subtree">
<t t-call="nodeTemplate">
<t t-set="node" t-value="subtree"/>
</t>
</t>
</div>
```
Where we want to propagate a recursion index through recursive t-calls
Before this commit, it did not work as we protected the scope in order
to not leak, in the wronf manner. Namely the protected scope only took
firt level prototype properties of the original scope.
After this commit, this case works as we mark the scope as read only
solves #672
In this commit, we uses the "!" as suffix to designate an event that
should be captured. This is inspired by the Vue source code.
This solves the issue of communicating additional information to the
underlying virtual dom. However, this will most likely disappear when
we rewrite the vdom as a virtual block system.
closes#650
Samuel Degueldre