Currently in some cases, adding an attachment via lognote creates a traceback.
Error : shouldPatch is true while `vnode` is not defined, so `patch()` failed
This is a hotfix correcting this problem by calling `patch()` only if `shouldPatch` is true **and** the `vnode` is set.
opw-2645203
This is a tricky commit. The key point is that the Fiber.complete
method, which commits a rendering to the DOM works like this: it
traverses the component tree, patch the corresponding DOM for each
component, calls the mounted/destroy hooks, and reset the currentfiber
of components to null, all synchronously.
However, this means that it is possible for components to initiate a
rendering (which create a new currentFiber) before the currentFiber is
reset to null, so the internal state of owl is corrupted. This can
occurs in a crash, as in the test that accompanies this commit.
To fix this, we take care of resetting the currentFiber first, while we
walk the component tree. Then, the internal state is always consistent
(i.e. a currentFiber to null means that there is no pending rendering)
closes#904
Before this commit, the error handling code simply destroyed the
application whenever an unhandled error occured in the owl rendering
process. This is perfectly fine, except that since the application is
potentially corrupted, the destroy code may crash as well. We simply
catch those errors to avoid shadowing the main issue.
closes#866
Before this commit, the error handling process was too naive: once an
error occurs in a rendering, owl catches it, looks for a component that
implements the catchError method, then calls it.
However, in real life, we sometimes need to rethrow that error (or
another one) to propagate the error to some parent handler. This error
needs to be handled by the closest parent component that implements
catchError.
This is what this commit implements: it wraps the catchError call in a
try/catch, then in case of errors, try to handle it by a parent.
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.
This commit tries to improve the interactions involving unmounted
components, or components mounted in an htmelement which is detached
from the main DOM, and rendering actions.
The main example is mounting a component in detached div, to prepare all
children. If we just mount the component, it will work as expected: the
full component tree is rendered in memory, and ready to be really
mounted at the desired target.
However, if before doing that, we update the component and call render
on it (for example, with a change in an observed state), then this
rendering will be ignored, and therefore, the full subcomponent tree is
not uptodate.
This commit will also solve another issue in the compatibility layer in
odoo: in the form renderer, we mount components with the adapter in a
div, which is not yet attached to the DOM. We then manually call the
mounted hook when on_attach_callback is called. This means that before
this commit, any change to the components between the initial rendering
and the call to mounted will be ignored.
As a bonus, this commit has the effect of bringing closer the semantics
of render and mount operations, which is certainly good.
closes#823
In some cases, a rendering initiated in some component is then remapped
into a larger rendering initiated by some parent.
If we have some components which implement shouldUpdate to return false,
then the following scenario can happen:
- some parent component is mounted (which triggers a rendering with
force: true => bypass the shouldUpdate)
- some sub component is updated and rerendered, AFTER the previous
rendering goes through it
- the sub component notices that there is an ongoing rendering, and
remaps itself in the parent rendering
Before this commit, the new fiber in the subcomponent does not have
force flag set to true, so the new rendering for the subcomponent does
not go through its own children (if they have shouldUpdate=false)
Another more complex kind of scenaria can happen when a remapped
rendering happen with sub components with dynamic shouldUpdate. The
problem is the same at the end: the new rendering should ignore the
shouldUpdate, to make sure we have the last correct information.
With this commit, we make sure that the flag of the new fiber is set to
true.
closes#818
Here is a situation that can happen in some complicated case:
1. a parent component is rendered, which includes some children
2. it is then willPatched
3. the sub components are then mounted/willUnmounted
4. because of complicated business logic, this causes the parent
component to be rerendered (before parent "patched" method is called)
5. owl will internally reset its currentfiber to null (but there is a
pending rendering!)
6. subsequent rendering will ignore pending rendering
7. havoc ensues
This is actually one of the reason why modifying a component state in a
willPatch component is actually not a good idea. However, the good news
is that this specific situation can be properly handled: we can simply
make sure that we do not reset currentFiber to null if there is a new
pending rendering.
closes#728
Have a hierarchy of A, B, C components where:
```xml
<div t-name="A">
<div>
<B t-key="key1"/>
</div>
</div>
<t t-name="B">
<C t-key="key2"/>
</t>
<div t-name="C">
<div><t t-esc="keys_as_props" /></div>
</div>
```
The subtility of the issues lies in B, which doesn't have its own
concrete DOM element, rather, it borrows it from C.
With the sequence of events:
- change key2
C1 is destroyed and replaced by another instance, and another node.
B1 has its props updated and is patched with the C2's node (CRITICAL)
A1 is patched
- change key1 AND key2
C2 is destroyed
B1 is destroyed
A1 is patched replacing B1 by B2, and their nodes too (which at this point should be C2's to C3's)
Before this commit, at the CRITICAL point, the node representing the component itself
(technically its pvnode) was not updated with the new concrete node provided by B1 patch with C2 node
i.e. it held the previous node still
The second array of steps crashed because at A1 patch, the new B2 node would replace B1, which
was out of the DOM (removed because C1 was destroyed long before),
and therefore without a viable parent to insert B2 node.
After this commit, we update the component's pvnode after the patch which elm had possibly changed
There is no crash anymore for this use case.
Have
```xml
<body t-name="webclient" />
```
and
```js
const comp = new WebClient();
comp.mount(document.body, {position: 'self'});
```
Before this commit, the body that was there before anything had happened
was *replaced* by the new body node created by the WebClient OWL component
After this commit, we ensure that the element body is the same at reference level
In most cases, we just want Component<any, Env>. But since it was so
annoying to have always the type Env, we actually used
Component<any,any> everywhere.
With this commit, the generic types have a default (and their order is
swapped), so we can simply use Component in most cases, and
Component<Props> when we want to type the props.
This commit is a significant refactoring of the internal of QWeb. It
simplifies the way variables/scoping and slots interact together. The
main idea is that we use a simple scope object instead of a
context/var/scope object.
This commit also implement the actual correct QWeb semantic for the
t-call directive with a sub body. Before, we simply extracted the
variables from the body and injected them at the top of the sub
template. We now simply compile the body before the sub template.
This is a joint work with Lucas (lpe).
closes#541closes#544closes#545closes#557closes#556
A component can now be mounted in different positions: either 'before' (at
the start), 'after' (at the end), or 'attach' (take possession of a target
element)
closes#539
The component Portal is used to teleport the content
in its slot as a child of an element
somewhere else in the DOM, usually 'body'
It is designed to be transparent and is just here to do the teleportation task
OwlEvents (with method `Component.trigger`) are redirected from
any Component instanciated within the Portal (and therefore teleported
somewhere else) to the place it would have been without teleportation
i.e. they are re-triggered onto the Portal root node
Co-authored-by: Aaron Bohy <aab@odoo.com>
closes#184closes#466
It is important to reset the isRendered flag to false to ensure
that other (subsequent) simultaneous calls to render will be
skipped, as the currentFiber is actually no yet (re-)rendered.
Closes#483
Following 2922cee6ea
Previous commit was not correct: used children with a cancelled
fiber (for instance, with a new currentFiber) could be destroyed.
This commit fixes the issue differently, by directly detecting
children that are not used anymore (and not mounted), and destroy
them directly (no need to wait for willStart promise to be resolved
anymore).
This rev. moves the logic of batching the notifications of changes
occurring in the same microtask tick, from the observer, to the
listeners (the context, and the render function of components).
By doing so in render, we ensure that similar renderings are
batched in a single one, wherever they come from (state change,
store change, direct call...).
Whenever a rendering is completed, we need to reset the currentFiber
to null to make sure all subsequent renderings will not be
confused.
However, the way it was done before this commit was wrong in a
specific situation: we resetted the currentFiber to null in the
patch method. The idea was that this method was called every time
the component completes a rendering. But this is not true: it
can happen that a component is unmounted and remounted without
changes. Then the component will be patched, but if there is no
change, it will not call recursively the patched methods of its
children.
So, we need to choose a better place to reset it to null.
closes#454
Achraf (abz)