[IMP] parser: add support for user-space directives #1651

jpp-odoo · 2024-11-07T12:27:00Z

This commit adds the support for user-space directives. To use the user-space directive, an Object of functions needs to be configured on the owl APP:

 new App(..., {
    userDirectives: {
     my-user-directive: function (el, value) {
            el.setAttribute("t-on-click", value);
            return el;
       }
   }
  });

The functions will be called when a user directive with the name of the function is found. The original element will be replaced with the one returned by the function.
This :

<div t-user-my-user-directive="click" />

will be replace by :

<div t-on-click="value"/>

issue : #1650

sdegueldre · 2024-11-07T14:30:19Z

If the user directives are defined on the app, doesn't that mean the entire feature is unavailable when precompiling?

kebeclibre · 2024-11-13T09:05:59Z

tests/app/app.test.ts

+    }
+    const app = new App(SomeComponent, {
+      globalValues: {
+        plop: (string : any) => {


not a fan of the API: calling __globals__.plop() is not natural.

Find a way at compile time to destructure that object on demand, a bit like helpers and bdom are.
or

rename every member of the globalValue to custom[Value]

Find a way to protect the helpers and globals from user alteration: if user's code accesses via __globals__.myfunc() and myfunc is not an arrow function, then myfunc could access, modify, add to the globals object (via its this)
Also, it is obviously a side effect of that feature, but it is another door open to memory leaks and errors due to scoping things into arbitrary, unkillable closures

What happens when:

A "main" app (let's the one spawning the webclient) defines such globals

a secondary app (lets say the ones spawned by the htmleditor) doesn't define such globals.
In the secondary app, it is legitimate to want to spawn the same components that the main app would.
Does the globals object gets shared, or do we expect a crash ?

there is no secondary app, that's why we have owl roots. they all share the same config/templates/translation function/settings and now global and user directives. should be fine

not a fan of the API: calling __globals__.plop() is not natural.

imo, it's not a problem, it will be extremely rare, it's only for some specific low level code

Find a way at compile time to destructure that object on demand, a bit like helpers and bdom are.

rename every member of the globalValue to custom[Value]
this is much much harder to do safely

Find a way to protect the helpers and globals from user alteration: if user's code accesses via __globals__.myfunc() and myfunc is not an arrow function, then myfunc could access, modify, add to the globals object (via its this) Also, it is obviously a side effect of that feature, but it is another door open to memory leaks and errors due to scoping things into arbitrary, unkillable closures

good point about memory leaks, i am unsure, need to think about it. now, i don't see how to really protect the globals object. maybe we can use Object.seal. could be a good idea indeed.

it is another door open to memory leaks and errors due to scoping things into arbitrary, unkillable closures

I for one am just not seeing it, mind expanding on that?

It was just a general observation. In the history of what we did in odoo, we realized progressively that when passing callbacks everywhere we take the risk to have some objects in a closure that can hold the whole OWL Tree. In this particular case, those closures would be, indeed unkillable. But I don't think they will hold much information......

src/compiler/parser.ts

src/runtime/template_set.ts

This commit adds the support for custom directives. To use the custom directive, an Object of functions needs to be configured on the owl APP: ```js new App(..., { customDirectives: { test_directive: function (el, value) { el.setAttribute("t-on-click", value); return el; } } }); ``` The functions will be called when a custom directive with the name of the function is found. The original element will be replaced with the one returned by the function. This : ```xml <div t-custom-test_directive="click" /> ``` will be replace by : ```xml <div t-on-click="value"/> ``` issue : #1650

This commit, add a new configuration on the App: globalValues. It's a global object of elements available at compilations. For instance: ```js const app = new App(SomeComponent, { globalValues: { plop: (string: any) => { steps.push(string); }, }, }); ``` The plop function will be available at the compilation, so it can be used on the templates : ```xml <div t-on-click="() => __globals__.plop('click')" class="my-div"/> ```

ged-odoo · 2024-11-22T10:03:05Z

src/runtime/template_set.ts

@@ -42,6 +47,8 @@ export class TemplateSet {
      }
    }
    this.getRawTemplate = config.getTemplate;
+    this.customDirectives = config.customDirectives || {};
+    this.globalValues = config.globalValues || {};


this.runtimeUtils = { ...helpers, __globals__: config.globalValues || {}} ;

and remove this.globalValues

ged-odoo · 2024-11-22T10:03:36Z

src/runtime/template_set.ts

@@ -97,7 +104,8 @@ export class TemplateSet {
      this.templates[name] = function (context, parent) {
        return templates[name].call(this, context, parent);
      };
-      const template = templateFn(this, bdom, helpers);
+      const runtimeUtils = { ...helpers, __globals__: this.globalValues };


this is done everytime a component is instantiated. it would be cheaper to do it just once in the setup. so, remove this line

ged-odoo · 2024-11-22T10:03:56Z

src/runtime/template_set.ts

@@ -97,7 +104,8 @@ export class TemplateSet {
      this.templates[name] = function (context, parent) {
        return templates[name].call(this, context, parent);
      };
-      const template = templateFn(this, bdom, helpers);
+      const runtimeUtils = { ...helpers, __globals__: this.globalValues };
+      const template = templateFn(this, bdom, runtimeUtils);


const template = templateFn(this, bdom, this.runtimeUtils);

jpp-odoo force-pushed the master-add_user_directives-jpp branch 3 times, most recently from e0e4d78 to f2eab03 Compare November 7, 2024 14:13

jpp-odoo force-pushed the master-add_user_directives-jpp branch from f2eab03 to 53807d8 Compare November 12, 2024 14:21

ged-odoo marked this pull request as ready for review November 12, 2024 15:38

kebeclibre reviewed Nov 13, 2024

View reviewed changes

jpp-odoo force-pushed the master-add_user_directives-jpp branch from 6a19f9e to 545cb32 Compare November 13, 2024 10:42

jpp-odoo force-pushed the master-add_user_directives-jpp branch 2 times, most recently from 9a824a6 to 7c17b7f Compare November 21, 2024 13:17

ged-odoo reviewed Nov 21, 2024

View reviewed changes

src/compiler/parser.ts Outdated Show resolved Hide resolved

ged-odoo reviewed Nov 21, 2024

View reviewed changes

src/compiler/parser.ts Outdated Show resolved Hide resolved

ged-odoo reviewed Nov 21, 2024

View reviewed changes

src/compiler/parser.ts Outdated Show resolved Hide resolved

ged-odoo reviewed Nov 21, 2024

View reviewed changes

src/runtime/template_set.ts Outdated Show resolved Hide resolved

jpp-odoo force-pushed the master-add_user_directives-jpp branch 3 times, most recently from 987429c to 3c34085 Compare November 21, 2024 14:32

jpp-odoo added 2 commits November 21, 2024 15:38

jpp-odoo force-pushed the master-add_user_directives-jpp branch from 3c34085 to a96815f Compare November 21, 2024 14:39

ged-odoo reviewed Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP] parser: add support for user-space directives #1651

[IMP] parser: add support for user-space directives #1651

jpp-odoo commented Nov 7, 2024

sdegueldre commented Nov 7, 2024

kebeclibre Nov 13, 2024

kebeclibre Nov 13, 2024

ged-odoo Nov 13, 2024

ged-odoo Nov 13, 2024

sdegueldre Nov 13, 2024

kebeclibre Nov 21, 2024

ged-odoo Nov 22, 2024

ged-odoo Nov 22, 2024

ged-odoo Nov 22, 2024

[IMP] parser: add support for user-space directives #1651

Are you sure you want to change the base?

[IMP] parser: add support for user-space directives #1651

Conversation

jpp-odoo commented Nov 7, 2024

sdegueldre commented Nov 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment