Commit Graph

469 Commits

Author SHA1 Message Date
Géry Debongnie cb38d795f9 [FIX] slots: properly handle named t-slots inside named t-slots
Previous code naively handled nested t-set-slots: if a second named
slots was found, it overrode the first.

In this commit, we use a set to make sure that we only use the first
found t-set-slot node. Also, we ignore set-slots defined in a sub
components, because these slots are only relevant to the sub component
itself.

Note that it works as expected because document.querySelectorAll
performs a search depth first, so we will always use the named slots
closer to the parent element, in term of depth.

closes #682
2020-10-09 15:01:25 +02:00
Géry Debongnie 38c7ad9629 [FIX] t-model: support expressions with [ ]
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
2020-09-18 14:58:48 +02:00
Géry Debongnie d0c76c5854 [IMP] qweb: add indeterminate to special input properties
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
2020-09-18 13:43:25 +02:00
Géry Debongnie e032314739 [FIX] qweb: allow t-call on arbitrary html nodes
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
2020-09-18 08:39:57 +02:00
Géry Debongnie b2db7f21ed [FIX] observer: does not proxify promises
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
2020-09-17 13:51:10 +02:00
Géry Debongnie 7b8ac13d3f [REF] qweb: mostly revert fix with t-call and vars with body
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.
2020-09-17 09:44:08 +02:00
Géry Debongnie 06d852fcf9 [FIX] component: allow using vars with body as props
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
2020-09-17 09:44:08 +02:00
Géry Debongnie 7e4baf668a [FIX] test: timing issue
Before this commit, we artificially replaced in the tests the
requestAnimationFrame by a setTimeout, to actually increase the speed of
the tests.  However, this is not really a true replacement.  For
example, a real setTimeout can come before or after a real
nextAnimationFrame, depending on when/where it is requested.

Also, this change exposed another problem: the nextTick function did a
setTimeout before a nextanimationframe. This is not a problem when
nextAnimationFrame is replaced by a setTimeout, because then all
expectations holds in owl.  However, it is wrong: to get to the next
animation frame, we need to request an animation frame, and THEN wait
with a setTimeout.

closes #729
2020-09-17 09:18:05 +02:00
Géry Debongnie fe34ba00a6 [FIX] component: properly validate multiple props
Because of a "break" statement instead of "continue", the check for valid
props was stopping as soon as it met an optional props, which kind of
invalidate the whole system.

closes #717
2020-09-17 08:33:18 +02:00
Géry Debongnie e5e7790530 [FIX] qweb: handle input value attribute as a property
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
2020-09-17 08:32:02 +02:00
Géry Debongnie 3bf91afc3f [FIX] qweb: fix issue variables set in body of t-call
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
2020-09-16 11:39:32 +02:00
Géry Debongnie 2529aa3ef2 [FIX] component: make concurrent renderings more robust
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
2020-09-16 09:21:33 +02:00
Géry Debongnie 8d25bddda4 [FIX] component: subtle issue with unmounted children
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
2020-09-15 15:51:30 +02:00
Lucas Perais (lpe) 81af21a025 [FIX] component, fiber: update props with virtual node should not crash
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.
2020-09-14 15:17:45 +02:00
Lucas Perais (lpe) 8e03f9cd9c [FIX] qweb: t-call should protect scope and let it accessible
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
2020-07-28 08:56:00 +02:00
Michael Mattiello (mcm) 9fd8315c53 [FIX] qweb: support of svg in t-raw
Before this commit, t-raw a string containing `<svg>` did not display
the svg element. This was because of missing namespace in vnode's data.

Now, `vdom.addNS` is called to add the svg namespace when necessary.
2020-05-25 14:02:34 +02:00
Géry Debongnie 85318b3ae6 [FIX] qweb: properly handle subtemplates in shared templates
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
2020-05-25 11:43:24 +02:00
Aaron Bohy 2af8cccd65 [FIX] qweb: t-esc inside t-call using outside t-set
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>.
2020-05-15 13:47:57 +02:00
Géry Debongnie b8d3618afa [IMP] hooks: improve useRef typings
With this commit, we can get the type for the target component for a
ref.

By default, if the generic type is not given, the ref will simply use
the base Component type.
2020-04-30 16:23:36 +02:00
Géry Debongnie 559fadb62a [FIX] component: properly handle mounting a destroyed component
part of #685
2020-04-22 09:29:39 +02:00
Géry Debongnie c36333dbbc [CLEANUP] update prettier to v2.0.4 2020-04-21 16:23:31 +02:00
Géry Debongnie b5c3422b4d [FIX] qweb: do not override t-att-class with class
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
2020-04-21 16:23:31 +02:00
Géry Debongnie 1707bd240d [IMP] add browser bindings to standard environment
This could be done by each application, but it does cost only a few
lines of code, and it helps standardizing the Owl ecosystem.

For example, some library (such as o_spreadsheet) needs to mock side
effects, and Odoo also needs to do that, so this prevents duplicated effort.

closes #686
2020-04-17 13:39:46 +02:00
Géry Debongnie 142b69823f [IMP] types: do not make Env an indexed type
Before this commit, Env was an indexed type, this means that one could
write env.anything, and it would accept it as a valid type. This is
actually quite dangerous, because we lose the typing advantages for all
keys that are properly defined.

For example, if a component is defined as:

class MyComponent extends Component<Props> {
 ...
}

Then Typescript will let it use anything from the environment, even if
it is wrong.  So, most properly typed Typescript applications should use
instead a sub environment:

interface MyAppEnv extends Env {
  someKey: someValue
}

Then, the component should be defined this way:

class MyComponent extends Component<Props, MyAppEnv> {
 ...
}

Before this commit, any typos in the environment accesses would not be
noticed by typescript.
2020-04-17 13:39:46 +02:00
Joseph Caburnay 94c8bce810 [IMP] allow anonymous Component extensions
It is possible that a Component is extended dynamically and if this is
the case, the class that extends it can be anonymous, with property
name=''. If this is the case, current implementation interprets the empty
string to be false so the while loop is terminated without further
scanning the super classes.

In this proposal, we allow anonymous class to be scanned until its
Component ancestor. Basically, the anonymous class assumes the name of
it super.
2020-04-15 08:54:02 +02:00
Géry Debongnie ae172d42e7 [IMP] slots: add new t-set-slot directive
This new t-set-slot directive is meant to replace t-set when we need to
define the content of a sub slot. All new code should use that
directive.

The old t-set directive is still supported for now, but this should be
removed when we publish Owl 2.0.
2020-04-06 15:42:32 +02:00
Géry Debongnie ddf30a8a97 [CLEANUP] run prettier on the codebase 2020-04-06 15:42:32 +02:00
Géry Debongnie 8d0d8538ad [FIX] slots: better heuristic to determine named slot content
Unfortunately, we chose to use the directive `t-set` to define sub slot
contents in a template.

The goal was to reuse a directive for a similar use case: defining sub
template is almost the same as defining a slot content.

Obviously, this introduces a name conflict: the inner content of a
component cannot use t-set t-value anymore (nor t-set with a body
value), since they are interpreted as slot names.

We mitigate the issue here by only interpreting as slot content the
`t-set` statement located immediately below the parent component tag
name and with a body content.

However, a real fix need to introduce an additional directive to resolve
the ambiguity.
2020-04-06 15:42:32 +02:00
Joseph Caburnay 2b0315c03f [FIX] hooks: order of brackets for multiple async hooks 2020-03-30 08:46:16 +02:00
lucas c469cac315 [FIX] qweb: protect scope in t-call writing recursively
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
2020-03-12 13:15:28 +01:00
Lucas Perais (lpe) b4ad14edc0 [FIX] component: self mounting position keeps the reference
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
2020-03-05 16:35:33 +01:00
Aaron Bohy 718e5264ae [FIX] test: missing await 2020-03-03 10:42:22 +01:00
Aaron Bohy f5d019bb69 [FIX] component: concurrency issue with cancelled fiber
Since ee956a197, the rendering is skipped if the currentFiber is
completed. Unfortunately, cancelled fibers remain set in __owl__,
so when a fiber is cancelled, subsequent calls to render are
skipped.

It would be nice to reset __owl__.currentFiber to null when the
fiber is cancelled, but when trying to do so, a lot of tests fail.

Part of issue #622
Closes #665
2020-03-02 11:35:47 +01:00
Géry Debongnie ce052e0992 [FIX] transition: no crash if added/removed quickly
This commit should fix crashes coming from transition code on nodes.
This does not impact transitions on components.

closes #637
closes #641
2020-02-21 09:30:47 +01:00
Géry Debongnie b00188c1ce [IMP] qweb: add support for event capture
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
2020-02-21 09:22:43 +01:00
Géry Debongnie 2c01802b8e [FIX] qweb: support attributes with quotes
closes #651
2020-02-21 08:08:55 +01:00
Géry Debongnie 20eb848262 [FIX] component: merge hooks properly
Before this commit, creating a sub component with
t-att-style/t-att-class attributes or with t-on- event handlers would
override the *hook* object rendered by the sub component.

This is an issue for some directives, such as t-ref, which defines
hook functions.

This commit fixes the issue by checking for an override, and wrapping
the hooks in a function that calls each defined hook.

closes #638
2020-02-17 15:50:53 +01:00
Géry Debongnie f7728b93bd [FIX] component: cancel previous mounting operations if necessary
closes #626
2020-02-17 12:49:32 +01:00
Géry Debongnie 1d7503913e [FIX] qweb: ignore comment nodes between t-if/t-elif/t-else
closes #636
2020-02-17 08:10:20 +01:00
Géry Debongnie ee956a1977 [FIX] component: resolve a subtle concurrency issue
Asynchronous rendering is subtle. Before this commit, it was possible
(though not easy) to get into a situation where the Owl rendering
pipeline decremented twice the counter of a fiber that indicates that
its work is complete.

The situation occurs in the `render` method, where we create a fiber,
wait for a micro tick before starting the actual rendering (the goal was
to batch all changes coming from a single call stack). However, in this
microtask tick, it was possible that the fiber was cancelled, and we did
not have a check for that.
2020-02-07 14:19:43 +01:00
Géry Debongnie 3fdc7a48f3 [IMP] types: make component generic types optional
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.
2020-02-06 09:37:44 +01:00
Géry Debongnie 1c8dc97fca [FIX] component: can mount on different target without unmounting
closes #616
2020-01-21 09:26:38 +01:00
Géry Debongnie f9cac94dc7 [REF] tests: split component.test into multiple files 2020-01-21 09:26:38 +01:00
Géry Debongnie 54f5819ef9 [ADD] hooks: add useExternalListener hook
It is very useful.

Also, this commit prettifies the code.

closes #608
2020-01-09 14:06:30 +01:00
mcm-odoo 74bd119bf5 [IMP] styles: handle & selector
This commit adds the support of the & selector.
This selector is useful to join a subrule with a parent selector.

example:

button {
  &:hover {
    background-color: red;
  }
}

will give

button:hover {
  background-color: red;
}
2020-01-09 09:37:27 +01:00
Géry Debongnie 4a0d04e4ae [FIX] debug tools: do not crash on non json-stringifiable props
closes #595
2020-01-09 09:24:50 +01:00
Géry Debongnie 49931104a1 [CLEANUP] update snapshots 2020-01-08 16:05:22 +01:00
Lucas Perais (lpe) 21d1306ec2 [FIX] qweb: t-foreach needs to hold on scope
Have a t-on within a t-foreach
the t-on has an expression

Before this commit, when triggering the t-on, the expression was falsely evaluated
In details, the expression took scope from outside the foreach loop
as we protect it to have similar results than an actual for loop

But, at triggering time, the scope protection had already been terminated
meaning the info of the iteration of the loop the handler has been built in
had already disappeared

This commit fixes that

closes #594
2020-01-08 15:45:44 +01:00
Lucas Perais (lpe) 21737d33fa [FIX] qweb: variables set outside foreach must be altered by inloop t-tset
In a template, have t-set t-value outside a t-foreach
in the t-foreach, alter that variable by resetting it (as for a incrementation variable)

Before this commit, when printing the variable when the loop had finished
its value was the one set in the first place

After this commit, the value becomes the one altered by the loop iterations

closes #598
2020-01-08 15:45:44 +01:00
Géry Debongnie 23012f3e7c [FIX] component: multiple t-calls
Before this commit, owl could crash in some specific situations:
multiple t-calls with sub components, outside a loop.  The reason is
that the t-call did not generate a key, so from the point of view of the
children component, it had the same key, even though it was at a
different place in a template.

Also, with this fix, we can fix the playground responsive example.

closes #602
2020-01-08 15:24:43 +01:00