In #1161, we fixed blockdom treating 0 as falsy when patching a value,
which would result in an empty attribute. It turns out that there is
another place where we had the same fallback, which is when we compute a
dynamic attribute value, so while blockdom had the ability to set the
value to 0, when using a dynamic attribute value we would never give it
0 as a value. This commit changes the fallback strategy for dynamic
attribute values to be the same as the one in blockdom.
closes#1358
Have a t-ref on a DOM node with a t-key.
Change the t-key.
Before this commit, the old block removed its element from the component's refs *after*
the new block had been mounted, meaning that in effect, the resulting
ref at the end of the whole patch was null.
After this commit, we only remove an element from the component's ref if that very same
element was indeed the ref. (otherwise it means someone else has changed the ref.)
In dev mode, owl inserts an additional props validation step. Before
this commit, the validation would iterate on all key/value pairs of each
props. If the props is reactive, this has the unfortunate side effect of
subscribing the component to each of the keys, even though it should not
be.
This commit avoids the issue by only validating the raw object.
Previously, the getSubscriptions debugging utility returned all observed
keys for a given target, instead of only the keys observed by the
callback it received as argument. This commit fixes that issues.
Previously, if the value of a t-component directive changed, but there
was already a scheduled render for the component before that change, the
rendering of the existing component would get delayed (as it should),
but after the parent's rendering was complete, the old component which
will be destroyed during the patch would still get rendered, despite
being stale. This can cause crashes if that component relies on state
that no longer exists.
This was caused by the fact that when rendering, we check the parent
chain for an active render, and also check that the component still
exists in the parent's fiber childrenMap (ie, we know that the upcoming
patch is not about to destroy the component). To do this, we use the
component's parentKey, but in the case of t-component, the parentKey for
both possible components is the same, causing the stale component to
incorrectly believe that it's not about to be destroyed, by finding the
next component in the childrenMap under the same key.
This commit fixes that by prepending the component's name to the
parentKey, meaning that if the value of the t-component changes, so will
the key, and the render will be further delayed until the new state is
patched, at which point the stale component will be destroyed and the
following attempt to render will be cancelled as expected.
The choice to use the component's name is to simplify the
implementation, this means that if using t-component and switching
between components that have the same class name, this protection will
not work. The alternative would be to generate a unique id for the
component class, eg with a WeakMap, but this both complicates the
implementation and adds runtime overhead, for a case that is likely
extremely rare. We are open to refine the implementation should this
problem occur in practice.
Have a radio group defined inside a t-foreach:
```xml
<t t-foreach="values" t-as="val" t-key="val">
<input name="radiogroup" t-att-value="val" t-model="state.radioGroup" />
</t>
```
Before this commit the algorithm that set the "checked" attribute on the current active
radio button according to the state did not support having a dynamic value (`t-att-value`)
After this commit, this use case works as we go look in the dynamic attributes too.
t-call-context is a feature that's supposed to mask the rendering
context completely, but currently the component remains available
through `this`.
This commit stops treating `this` as a reserved word, so that it's
compiled to a lookup in the rendering context, and adds `this` to the
rendering context when binding the component's rendering function. With
these changes, `this` behaves the same as before when outside a
t-call-context, but when the rendering context is overriden, the
template can no longer access `this`. `this` still represents the
instance of the component inside of the rendering function since it's
needed by owl internally.
A side-effect of this change is that now the rendering context is no
longer the instance of the component by default, but is always an object
with the component in its prototype chain. This was already the case
before in some contexts (eg inside t-foreach, or inside components with
a t-set/t-call anywhere in its template). This can cause issues in rare
cases when a component method was called directly on the rendering
context, as before this change, the method's bound this would be the
component instance (except in a t-foreach, component with a
t-set/t-call, etc), while after this change it is now never the
component instance. When the method only reads on `this` there is no
issue as all the components properties are available on the rendering
contexts, but setting a value on `this` will write on the rendering
context and not the component which is likely a mistake.
While this is a breaking change, simply adding a t-set/t-call to any
template would break components that would be broken by this change,
with this in mind we decided to make this change anyway so that
developers get the error as early as possible in the development cycle
rather than having a seemingly inocuous change break code under them.
Previously, when using a component with a slot within a t-call with
t-call-context, the component would become available again inside the
slot despite the t-call-context. This was caused by the fact that the
capture helper function creates an object with the component as its
prototype which is incorrect. It should just use the previous context as
its prototype.
With this commit, we make sure that the code for t-model is run before
t-on-input event handler. This is useful to make sure that the state
reflected by t-model is up-to-date.
closes#1295
With the introduction of t-call-context, there is now no guarantee that
the ComponentNode can be found in the rendering context, any attempt to
do so can crash when combined with t-call-context. This commit fixes
that by using `this` instead, which in compiled templates refers to the
component that is being rendered.
When a parent component has an empty child with a t-on
and an event is triggered inside the parent, blockdom
checks if there is an event_catcher to call.
Doing so, an empty child would cause an error.
This commit fixes that.
Currently, the `moveBefore` method on VNodes assumes that the `other`
VNode it receives is of the same type, and that the entire VNode tree
below that other VNode has the exact same structure. While this is
correct in most cases, it breaks down when there is a VToggler somewhere
in the VNode tree, as the structure below a VToggler can be very
different from the structure below another VToggler that was created
from the same compiled code. For example, two iterations of a t-foreach
that contains a <t t-component="..."/> may spawn different components,
and different components obviously have different structures.
One way to fix this is to remove the assumption that the structure of
the `this` block tree in moveBefore is the same as the structure
of the `other` block tree, and instead, always give the concrete DOM
node before which we want to move the current VNode instead of giving it
a VNode and an afterNode as a fallback. One problem with this solution
is that it degrades performance in the "standard" case, where a
t-foreach contains no VToggler anywhere in its block tree, as retrieving
the first concrete DOM node requires calling firstNode() which
recursively traverses the entire tree.
To avoid this performance penalty in the standard case, we opt to only
go down this route whenever we encounter a VToggler when calling
`moveBefore`. This requires that we maintain two separate methods, one
to move a VNode before another VNode of assumed similar structure, which
is basically the current implementation of `moveBefore` for all VNode
types except VToggler, and one implementation that moves a VNode before
a concrete DOM node. This method needs to be implemented for all VNode
types, as all VNode types can be descendants of a VToggler. This method
will only be called from one place: the `moveBeforeVNode` method of the
toggler, which is the point where we realize that the assumption of
identical structure breaks down.
Co-authored-by: Bruno Boi <boi@odoo.com>
Currently, if a component has a default slot defined with t-set-slot,
and also content that compiles to something (eg, text or even a comment
node), the content takes priority over the t-set-slot. As t-set-slot is
more explicity, it should have priority.
Currently when checking for duplicate keys, we insert the value of the
key as is in a set then check for unicity against those. When the key is
an object, we check for duplicates based on object identity, whereas the
keys are used by owl as strings, and so using objects can cause
duplicate key errors that do not throw correctly but crash in the owl
internals.
This commit fixes that by making the duplicate checking code serialize
the key to string before insertion and when comparing against existing
keys.
The t-out directive is compiled internally into a LazyValue, which
represents a value that may or may not be created sometimes in the
future. It can also be reused more than once, and this is where there
may be an issue: if a component is contained in the lazyvalue, it needs
a unique key (coming from the t-foreach) to be properly indexed in the
parent children map. However, the LazyValue does not keep the key
information, so it is not able to provide it to its content.
The fix is then quite clear: the LazyValue class should store the key
information, and provides it to its content. This allows the LazyValue
to be used multiple times, in any place in a template.
closes#1270
A recent commit fixes the props validation code to make it work
regardless of the rendering context (important with the recent
t-call-context directive). Unfortunately, it then breaks props
validation through slots, because it assumed that the parent node in the
virtual node was the parent of the component, but it is not necessarily
true.
To fix this, we can use a simple property of the template functions:
they are bound to the current instance of the component, so we can
simply use "this"
The code for props validation assumed that the rendering context was a
component. This was actually true when it was written, but is no longer
true since t-call-context was introduced.
Because of that, it would crash when trying to access the internals of
the component, such as the static components object.
The fix is simple: instead of passing the context to the props
validation code, which can now be anything, we pass the component node,
which is guaranteed to give a reference to the component (and also to
the app). This also make the code slightly simpler.
closes#1261
Previously, event handlers would not work when an app was mounted in an
iframe, this is caused by a guard in the event handler that checks that
the target element is still in the document, but it doesn't check
against the correct document in the case of an iframe.
This commit changes the check to check against the target's
ownerDocument.
Before this commit, a crash could occur when a component with no props
is defined in a slot, and that slot is conditionally displayed in
multiple locations.
The reason for that is that the key provided to the callSlot function
was identical, so from the perspective of the component function, it was
not possible to make the difference between a component located in
either places. With this commit, we make sure that a unique key is used
when a slot is reused in a template (or if it is dynamic, because in
that case, we have no idea at compile time if it will be unique or not)
closes#1246
Previously, when wrapping errors in wrapError, if the error was not an
actual error object, we wouldn't set the cause property on the wrapping
error correctly. The "instanceof Error" check is simply there so that we
can know whether we can add the original errors message to the wrapping
error, but the line that sets the error's cause was mistakenly moved
into that condition.
This commit also fixes the wrapping error's message in the case of
non-Error objects, to avoid having "the following error occurred in
hookname:" with nothing after the colon which is confusing/misleading.
This commit makes all errors thrown in owl use a custom error class. The
main point of this is to always wrap user-code errors that happen during
the owl lifecycle so that they can be treated uniformly in onError by
checking the cause property, and also allows user code to differenciate
owl errors from non-owl errors reliably at runtime.
Before this commit, wrapping an error occurring in async code
would result in an unhandledpromise exception, because we created
another promise, that would be rejected and that we didn't catch.
Before this commit, whenever Owl encounter a t-if, it generates an
anchor (a "hole") in the current block being compiled. However, in some
cases, the content of the t-if may not have any content at all, and the
anchor is then useless. Worse, the code generating the anchor generates
an index based on the number of sub blocks, but if there is no content,
the next anchor being created will have the same index, which then may
cause weird bugs.
A possible way to fix this could be to make sure we increment properly
the anchor index, but we could even do better: not having an anchor at
all.
Before this commit, the generated code for the component directive was
using the current context as the place to look for static informations
(such as the sub components). However, it is not entirely correct, since
the current context may be different than the current component (which
is easily accessed by using the this variable).
Also, while doing this, we fix some issues in the t-set directive, which
as calling lazy values with the wrong this.
Before this commit, using t-on on t-component was not correctly
implemented by owl: when more than one component shared the same
parentelement and have an handler with the same name, the second handler
would override the first (using an internal key). With this commit, we
simply recreate event handler for each instance of a catcher block. Note
that it means that using t-on on components is slightly slower now.
Also, this commit fixes an additional issue that was noticed: the
catcher block would not update its internal handler properly, so it
would not behave properly after being patched.
closes#1199
Before this commit, Owl had a bad interaction with the static
defaultProps code. The props comparison would be done with the new props
object (unmodified) and the current props object (with default props
applied), so the comparison would always return false, which in turn,
causes additional useless renderings.
This commit modifies component node to keep a reference to the
(unmodified) props object so we can compare it as expected.
Previously, components would automatically call useState on their props,
so that changes deeply within props would automatically cause the
component to be rendered. This can be useful when passing a piece of
state to children or descendants.
One problem with this is that all props implicitly become reactive, even
if the object that was passed as a props was not. The problem with that
being that since the original object is not reactive, any change made by
the parent will not go through the reactivity system and the children
won't be notified of the change, in essence, this reactive object is
essentially useless, while having a real cost: traversing reactive
objects creates more reactive objects, and those objects are all
proxies. This is expensive for basically no benefit, while also making
it more difficult to debug code that involves those objects.
This commit fixes that by only calling useState on objects that are
already reactive, allowing the usecase described in the first paragraph
without the drawbacks described in the second.
If you declare a child component which is not actually a Component,
the error message is not very friendly and not very helpfull to find
what happens and which child component is wrong.
```
const ChildComponent = "not a component constructor";
class MyComponent extends Component {
static components = { ChildComponent };
}
```
This commit improves the type declaration for those working with Typescript
and adds a runtime check for javascript codebases
Before this commit, there were 2 different ways of generating variable
identifiers. And it was possible to have a situation with two different
variable in a template with the same identifier, which caused a crash.
This commit ensures that we go through a unique helper method, so this
cannot occur anymore. Also, the code is slightly simpler.
Previously, if a fiber was delayed because one of its ancestors was
rendering, and that fiber was not a root fiber, it would be rendered
after all its ancestors had finished rendering even if one of those
ancestor renderings cancelled it.
This commit fixes that by simply checking that a delayed fiber is still
its component node's current fiber before rendering it.
With this commit, we make sure that the `render` method was explicitely
called with the deep === true argument, instead of assuming that it is a
boolean. This prevents errors when some unrelated value is given to the
render method. This could happen in some cases, such as
useBus(someBus, 'someevent', this.render)
In that case, the `useBus` code would simply call the this.render with a
customevent, which would be considered truthy before, and not anymore
Before this commit, the list of all children was managed at the level of
the root fiber, but this could cause issue when subfibers would be
reused. With this commit, we use the childrenMap object that exists on
each fiber instead.
Canceling a fiber may cause user code to be run, which means that some
new renderings could be scheduled, but this could interfere with the
current renderings!
A little typo was preventing a dynamic t-slot with a scope
(`<t t-slot="{{ state.name }}" myScope="someValue" />`)
to work properly.
This commit corrects this.
Before this commit, a t-set-slot that has no content was not even compiled, and thus not passed
to the component for which it has was defined
After this commit, we allow a t-set-slot to have no content (because it can have slot props)
This is a breaking semantic change. With this commit, the UI is frozen
whenever owl is waiting for a parent to change
Also, this allows Owl not to render components that will be removed
later.
This commit adds a warning when an async hook
(onWillUpdateProps/onWillStart) takes longer than 3 seconds, as these
hooks block the rendering and patching of the application, it is rarely
desirable and often a sign of a deadlock. This warning will contain the
stack trace of the call to the hook to help in debugging.
Before this commit, if the effect function would throw, then the cleanup
function would not be properly assigned, which caused additional errors
later, when the cleanup code would try to call it.
closes#1149
Samuel Degueldre