search

deftjs / DeftJS5

Deft JS for Ext JS 5
MIT License
34 stars 13 forks source link

Problem with ExtJS 5 and chart series component #9

Open filippopossenti opened 9 years ago

filippopossenti commented 9 years ago

When using DeftJS in a project containing charts, you may experience an error like the following:

Uncaught TypeError: Cannot read property 'incr' of undefined privates.doAddListener @ ext-all-debug.js:12630 addListener @ ext-all-debug.js:12384 Ext.Base.Ext.apply.createAlias.aliasOneMember.(anonymous function) @ ext-all-debug.js:7317 Ext.define.register @ deft-debug.js:771 (anonymous function) @ deft-debug.js:800 Ext.Function.ExtFunction.interceptAfter.object.(anonymous function) @ ext-all-debug.js:4638 Ext.define.constructor @ sencha-charts.js:1 constructor @ ext-all-debug.js:7657 ...

According to my present investigation of the issue, it's related to the Ext.chart.series.Series object in ExtJS that apparently invokes Ext.ComponentManager.register before initialization of the Observable mixin and despite not being an actual Ext.Component instance.

Even though it's unclear whether this could be considered a bug in ExtJS (registering as component something that it is not) it's probably still unsafe for DeftJS to assume that the Observable mixin is initialized when Ext.ComponentManager.register is called.

Proposed solution would be to add the possibility to specify a flag on target components to avoid registration by Deft.event.LiveEventBus.register, more or less as follows: Ext.Function.interceptAfter(Ext.ComponentManager, 'register', function(component) { if(component.registerInLiveEventBus === false) return; Deft.event.LiveEventBus.register(component); }); Ext.Function.interceptAfter(Ext.ComponentManager, 'unregister', function(component) { if(component.registerInLiveEventBus === false) return; Deft.event.LiveEventBus.unregister(component); });

Alternatively, modify the Deft.event.LiveEventBus.register method to both check for the availability of "on" and "un" methods, as well as correct initialization of the Observable mixin, for example by checking whether "hasListeners" is defined and only then attach the events listeners for 'added' and 'removed'.

Regards

kryzys69 commented 9 years ago

Hi, Please check this post on DeftJS forum. IMHO it should be treated as a bug in ExtJS and it is muuuch easier to fix it in ExtJS code.

Chris

brian428 commented 9 years ago

It's definitely a bug in Ext.chart.series.Series to register the component before it is initialized.

That said, I have no idea if Sencha plans on fixing this (even though it's literally moving one line of code to the bottom of the constructor. So we may have to add guard logic to deal with it anyway.

filippopossenti commented 9 years ago

Since my needs are very simple, for the moment I put in place my own guard logic but it's a solution that cannot last for long of course.

As a side note: LiveEventBus, in creating an interceptor for the Ext.ComponentManager.register method, is actually tapping in "private" functionality of ExtJS, that is not guaranteed to keep existing nor is guaranteed to work in a certain way... which is why I say it's unclear whether this is a bug in ExtJS or not. Also, I'm not sure that for Sencha is simply about moving a line of code: in the Ext.Component code, the constructor to the Observable mixin is not called within the Ext.Component.constructor method and is, instead, delegated to the "beforeInitConfig" method... which is still unclear to me when is called. Furthermore, Ext.Component allows to prevent registration via a "preventRegister" property whereas Ext.chart.series.Series does not. It would seem to me that Ext.chart.series.Series is quite "peculiar" in a number of things, meaning this odd behaviour could be the result of implementation choices rather than pure oversight. Bottomline: it's in a "gray area" and I wouldn't rely on a fix from Sencha.

Regards

brian428 commented 9 years ago

In Ext JS 4, ComponentManager.register() was public, so this is another case where Sencha has retroactively broken the API, since this method is now private in Ext JS 5. Why this is now private is somewhat confusing, since up until 5 it wasn't uncommon for folks to call this in custom components.

That said, what probably really needs to happen is to determine if we even need the LiveEventBus any longer. My suspicion is we probably don't, since all the view listeners are now managed by the new Sencha event system. And I'm fairly sure the new system handles dynamic addition and removal of view components.