When defining a template with a name that the template set already
contains, we currently always check whether the template is the same and
throw an error when it's not. This is potentially expensive as it can
involve serializing a pretty large XML document. This check is only
supposed to help during development so this commit disables this check
outside dev mode.
Previously, support for iterables was added to t-foreach. The idea was
that anything that you can spread or on which you can use for..of would
be supported. Due to an implementation mistakes, strings, which are
iterable were not supported because we checked that the typeof the
iterable was 'object'.
To fix this, we coerce the iterable to an object and check whether that
coerced value has a Symbol.iterator property, which is what happens
behind the scenes when using for..of or spreading a primitive.
Closes: odoo/owl#1503
Firefox will not serialize the xmlns attributes of node inside
XMLDocuments, see https://bugzilla.mozilla.org/show_bug.cgi?id=175946
it will only output an xmlns attribute if the node being serialized has
a non-null namespaceURI.
When compiling templates, we currently use a special attribute,
"block-ns" to keep track of the namespace, but we do not make use of the
namespace to create the elements that will be serialized to the compiled
block string. This causes a difference in behaviour between Chrome and
Firefox: since we end up with an Element with two attributes: the
block-ns attribute and the xmlns attribute. Chrome will serialize both,
but Firefox will only serialize the block-ns because the Element does
not have a namespaceURI.
To fix this issue, we get rid of the block-ns magic attribute
completely, and use xmlns instead. We also use the namespace when
creating intermediate elements that will be used to create the
serialized block string: the namespace is only used to create the
elements but *not* set as an attribute, as this would cause chrome to
serialize it twice, causing a duplicate attribute error when parsing it
further down the line.
When creating the template element for the block at runtime, the xmlns
attribute is used both as the namespace with which to create the
element, and set as an attribute, this doesn't cause issues when
serializing later because the namespaceURI is never serialized as an
attribute when serializing an HTML document[1], so we avoid the
double-serialization in Chrome, and when doing HTML serialization,
Firefox will correctly serialize the attribute.
It's desirable that the xmlns is set as an attribute to allow users to
use owl to render SVG, and then use the HTML serialization of it as a
SVG even outside the context of an HTML document (eg to generate an SVG
file or an SVG data URL).
[1]: https://w3c.github.io/DOM-Parsing/#xml-serialization
Because the default content of a t-call is compiled before the block of
the t-call is created, the default content can sometimes incorrectly be
treated as though it is at the root of the template. This causes various
issues, in the case of a t-if, the default content will just replace the
t-call entirely when truthy, and when falsy the template will not return
anything, leading to a crash. Similar things occur with t-foreach and
t-out with a default content.
This commit fixes that by moving the block creation for the t-call ahead
of the compilation of the default content. In doing so, it makes the
context attribute "preventRoot" obsolete: the purpose of this attribute
is to somehow make the code aware that the root of the template will be
defined later, but because it wasn't propagated everywhere properly it
caused this issue. Now that the root already exists before we compile
the default content, we don't need it anymore.
In odoo/owl#1352 we added support for using t-foreach on Map objects,
maps should behave the same as objects where keys are available under
the name specified in t-as and values under that same name with the
suffix `_value`. Unfortunately, because the code for objects was written
in a confusing way (values were stored in a variable named `keys` and
vice versa), the implementation for Map was incorrect and keys and
values were swapped.
This commit fixes that and rewrites the code to be less confusing: keys
are extracted from the variables `k_block` and values from `v_block`,
which swaps the behaviour, and the code to prepare a list from an object
now extracts the keys in `keys` and the values in `values`
Previously, when a template failed to compile because of a syntax error
(typically because we don't do any syntax checking when compiling
expression, allowing invalid expressions to be transpiled successfully),
the error reporting was very minimal: you would only get the error
message itself (eg: "Unexpected token") with the stack information of
the error pointing to the call to `new Function` in owl, which is not
very useful.
This commit catches the compilation error and completes it with
information about the template name when available, and also adds the
generated code to the error message, allowing the user to just
copy/paste it in their web console or code editor to get a more precise
location for the error.
Before this commit, the template parser would allow using t-esc on a
component tag (<MyComponent t-esc="expr"/>) but would incorrectly ignore
the component when parsing a t-out: <MyComponent t-out="expr"/> would be
parsed as <t t-out="expr"/>
This commit solves the issue, and also, moves the `t-out` parsing code
next to `t-esc` so they have the same priority relatively to other
directives.
closes#1483
Before this commit, owl parser would ignore the `.number` suffix on
<select> options. I do not see a good reason for that, and it prevents
some legitimate usecases.
closes#1444
Class attributes are managed in a specific way in owl: they are merged
with existing class attributes, and also, they can be defined in
multiple ways (t-att-class, t-attf-class, t-att=['class', ...] and each
of these can be combined together.
Before this commit, the `t-att` syntax was handled as a normal
attribute, and therefore, would not combine as required with existing
classes.
closes#1453
Before this commit, the `t-on` directive assumed that the next character
would be a -, and ignored it, so if someone would write `t-onclick` by
mistake, Owl would then add an event handler on the `lick` event,
instead of `click`.
With this commit, we improve the parser to make it stricter and fail if
there is no dash.
closes#1441
Before this commit, the translate function was not applied to text
content inside the body of a t-set (unless that content was itself
inside some html). This is not clearly not intended.
To fix it, we just need to call the translate function at the
appropriate moment.
closes#1434
Before this commit, doing `t-out="null"` would crash, since the
safeOutput function would assume that it is a block, thanks to the fact
that `typeof null === 'object'`.
We handle here the null value just as if it was undefined: it outputs an
empty string.
It is common in Owl components to have anonymous function as props.
However, since each rendering create a different (but equivalent)
closure, Owl will consider the props different, so will update the child
component, but this is (often) not necessary.
This commit will help reduce the problem by introducing a new `.alike`
prop suffix, that will let Owl know that each version of that specific
prop should be considered the same, so, will be ignored by the props
comparison code.
closes#1360
Previously, refs could get set to null incorrectly, or could stay set
when they shouldn't. This was caused by the fact that singleRefSetter
and multiRefSetters are created on every render, meaning that any render
that did not affect the status of the ref (mounted or not), would
overwrite the previous ref setter, including its closure, causing the
captured `_el` or `count` to be lost. This means that on a subsequent
render that did affect the status of the ref, the singleRefSetter would
consider that it did not set the ref, and so shouldn't unset it, causing
it to incorrectly stay keep the element, while in multiRefSetter, the
opposite problem occured: the count would be 0 on removal, causing it to
believe that it was the first call in the corresponding patch that
affected the ref, when in fact, the closure is already stale, because it
was created from the previous render, and the fresh multiRefSetter from
the latest render may already have been called by an element with that
ref that came earlier in the template.
This commit changes the ref setting strategy to work around the issue of
stale closures: we define a setRef method on ComponentNode that is
always called, this method ignores calls with `null`, meaning that the
refs object on the ComponentNode never reverts to a null value for any
key. Instead, the useRef hook will check whether the element that the
ref points to is still mounted, and return null if not.
Before this commit, the parser would remove all consecutive white spaces
for text nodes. After that, the code generator would call the translate
function with the resulting string, which then is different than what we
would expect.
With this commit, we make sure we apply the translation before removing
the additional whitespace. To do that, we have to move the code
processing the string from the parser into the code generator, which
actually makes sense, as the parser should only collect all useful
information without applying too much logic.
closes#1351
Before this commit, Owl template compiler would handle readonly
attribute as readonly properties. But this is incorrect, since the
property is actually named readOnly.
With this commit, we make sure that readonly AND readOnly are both
interpreted as the `readOnly` property. This is due to the fact that
QWeb does not discriminate between attribute and properties, so we have
to infer which is which.
closes#1362
Before this commit, properties were just handled as a special case of
attributes. But it does not make that much sense, since they are
different. For example, the `readOnly` property does not have the same
name as the `readonly` attribute. The confusion probably comes from the
fact that QWeb does not distinguish between property and attributes, so
Owl has to infer which one is which.
With this commit, we introduce a `block-property` directive in blockdom,
and change the compiler to use it in emitted code. It has the additional
benefits of slightly shrinking the runtime code, since the `isProp`
function can now be located in the compiler.
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.)
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 a template contained backslashes, they were not escaped
when creating the blockstring, meaning they would be interpreted as an
escape sequence within the JS string. This means that backslashes
preceding most characters were completely ignored and didn't end up in
the final block, double backslashes were collapsed to a single one, and
backslashes that constituted valid escape sequences in JS would be
treated as those (eg, \n would be a newline).
When creating a JS string expression from a string value, all characters
with special meaning in JS should be escaped, we were correctly escaping
backticks as these would unexpectedly close the string if not escaped,
but forgot to escape backslashes. This commit fixes that.
closes#1300
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.
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
Before this commit, the template compiler would guess the next block id
that will be generated when compiling the body of a tcall. This is
correct IF there are not nested t-call, but otherwise wrong, because the
next block id could be mixed up: the first t-call would save the next
block id (let's say n), then the inner t-call would also save the same
block id (so, n), will then generate its own block (n+1), then the outer
t-call would use the block n index instead of n+1
The best fix, in my opinion, is to make sure we get the next block var
name, so we do not have to guess. To do that, each compile block type
function needs to properly return the information.
closes#1267
Recently, we made it so that when a component is rendered, it always
updates the property values for computed properties. This was done by
wrapping the value in a String or Boolean object. One issue with this is
that wrapping a falsy value in a String doesn't yield an empty string,
but a string containing the value as text (eg new String(undefined) ->
"undefined"), which causes the value to not remain empty as per the
spec. This commit fixes that by adding a fallback to the empty string
for falsy values before converting to a String object.
closes: #1236
Before this commit, owl would incorrectly skip patching html properties
when the new value is the same as the value used in the previous render.
However, this is incorrect, since the user may have changed the value by
clicking on a checkbox, or changing some text in an input.
So, to be more correct, owl has to force the update whenever it
encounters a prop. This is however hard to do without impacting the
main update loop, so we kind of work around the problem by using String
and Boolean instance, instead of primitive values.
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.
Previously, having a leading or trailing space caused
HTMLElement.classList.add to be called with an empty string (because we
are splitting on whitespace), which is not allowed and caused a crash.
This commit fixes that by trimming the keys of the class object in the
same way that we already do it for class strings.
Before this commit, the generated code crashed if an attribute in
the template contained backticks. The compiler output something
like
```js
let block1 = createBlock(`<div foo="`bar`"/>`);
```
The backticks are now properly escaped.
Previously, the support for the "in" operator was iffy, and "new" was
not supported at all, "in" would fail when checking if a nested property
was in something else, as the leading space would be stripped, and "new"
would fail because the following space would be stripped.
This commit fixes that by preserving the significant whitespace where
necessary.
Previously, we used an option to allow duplicate templates, if that
option was passed, we would always replace the existing template with
the new template, and if it wasn't, we would always throw an error even
if the template's content was the same.
Allowing to replace a template with a different one is problematic, as
it may or may not have already been compiled, which may cause various
problems. On the other hand, if the template is the same, there is no
point in throwing an error, as we can just silently ignore the second
addition.
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)
Samuel Degueldre