ibm-js / delite

HTML Custom Element / Widget infrastructure
http://ibm-js.github.io/delite/
Other
68 stars 28 forks source link

Upgrade to DCL V2 #498

Closed wkeese closed 6 years ago

wkeese commented 6 years ago

Upgrade decor, delite, deliteful, and wkeese/dcalendar to use dcl V2.

See http://www.dcljs.org/blog/2017/06/09/new-major-release-2-dot-0/ for information on the new(ish) release. Benefits include:

Possible stages of conversion

Switch to actually use dcl 2

Need to replace all references to dcl.mix(), which is no longer defined.

I hit a few bugs (uhop/dcl#25 and uhop/dcl#26) but they are now fixed.

Remove dcl v1 workarounds

Remove mixin classes added just to workaround https://github.com/uhop/dcl/issues/9

Leverage base classes / mixins / utilities

  1. Make decor/Stateful extend http://www.dcljs.org/2.x/docs/bases/mixer/ or http://www.dcljs.org/2.x/docs/bases/replacer to mix in property values

Note: decided not to do this because Stateful is explicitly defined to do the mixins after all the constructor methods have finished running.

  1. Use http://www.dcljs.org/2.x/docs/advices/memoize/ in places like Template.getElement() and Template.getProp()

  2. Make decor/Destroyable extend dcl/mixins/Destroyable.

Could also extend dcl/mixins/Cleanup (http://www.dcljs.org/2.x/docs/mixins/cleanup/), but I decided not to because Cleanup doesn't have code to address dangling references in this case:

this.pushCleanup(otherClass);
...
otherClass.destroy();

At this point otherClass cannot be garbage collected because this still has a reference to it. So we still need (some) of the advice stuff in decor/Destroyable.

dcl.prop()

Redesign Stateful API so subclasses explicitly declare monitorable properties?

This is the only big one, because it's an API change. It could be a lot of work but could also be a big performance boost.

It's based on http://www.dcljs.org/2.x/docs/dcl_js/prop/. The idea would be to declare classes like:

dcl(Stateful, {
     param1: Stateful.prop("param1", "initialValue1"),
     func1: function () { ... }
     ...

to indicate that observe() reports changes to param1, but not func1. And Stateful.prop() would simply be implemented something like:

Stateful.prop = function (prop, val) {
    return dcl.prop({
        get: function ()  { return this._get(prop); },
        set: function (val) { return this._set(prop, val); }
    });
};
uhop commented 6 years ago

Both issues in dcl v2 are fixed, and a new version 2.0.6 is published in npm.

BTW, there is another version of dcldcl6. It is a version for ES6 (classes, symbols, and so on). It is not published because there is no documentation, and tests for the debug module are unfinished. Yet, I use it already, and it should be understandable for users of dcl and, of course, there are tests.

BTW, some features of dcl6 were actually back-ported to dcl at some point or another.

wkeese commented 6 years ago

Thanks @uhop! I do know about dcl6 but I don't think we can use it because presumably it doesn't support IE11 (and we need to support IE11). Did I misinterpret?

It would be nice to use because the custom elements V1 API only works with classes. (That's according to https://github.com/webcomponents/custom-elements#es5-vs-es2015)

uhop commented 6 years ago

You are correct, but partially. The way to do it nowadays is to use the modern JavaScript supported by all modern browsers, and transpire it for the rest, e.g., IE11, with Babel. Last time I checked dcl6 works when transpiled and used with a standard Web Component shim.

wkeese commented 6 years ago

Ah that makes sense. Actually we are already transpiling, so I guess we could switch to dcl6, but I'll put it down for a "stage 2" because it's a big syntax change.

It might be nice though if dcl2 created ES6 classes behind the scenes, when ES6 was supported. Or are you already doing that?

uhop commented 6 years ago

It does use ES6 classes inside. Something like that:

const NewClass = dcl(BaseClass, Mixin1, Mixin2, Base => class extends Base {
  constructor() {
    super();
    // ... do stuff
  }

  method1() { /* ... */ }

  get prop1() { return this._prop1; }
  set prop1(value) { this._prop1 = value; }
});

The usual ES6 class stuff. Plus chains and AOP.

The reason I didn't publish it yet: there is an effort to bring decorators to ES6 classes. The discussion never ends. But I don't want to commit to something totally incompatible with the result. OTOH, for now, I decided to use a separate directive object, which lends itself to future decorators whatever they are. Of course, it is all predicated on them being not completely brain-dead.

Another reason is I didn't decide on packaging yet. Most probably I'll leave it to application developers and provide raw ES6 code.

Look at tests to get the feel. They are mostly ported from dcl v2.

wkeese commented 6 years ago

I was actually suggesting the dcl version 2 internally use classes (on supported browsers), so that it could also be used with CustomElementRegistry.define(). Not sure if it's a good idea or not.

uhop commented 6 years ago

Not doable. Unfortunately class is not a syntax sugar as popularly believed. It is the only way to subclass native objects like Array or HTMLElement. It is not possible to do by messing with prototypes in JavaScript. So I had to redo how mixins are defined and how chains/AOP declared. Essentially dcl6 is dcl v2 with classes and an updated API to facilitate the change.

You can think that dojo.declare() was for ES3, dcl was for the incomplete ES5, dcl v2 is for the complete ES5 (+ properties), dcl6 is for ES6 (+ classes, symbols, more introspection).