sunjavagroups / flot

Automatically exported from code.google.com/p/flot
MIT License
0 stars 0 forks source link

Unnecessary loading of plugin code #584

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
I have several charts loading on a single page, and flot calls initPlugins for 
each one. One of my plugins (customized selection) contains a lot of methods 
and it creates a listener for a plotresize event (another plugin). This 
listener was firing on every single chart, even though only one of my charts 
was actively using my plugin. 

For complex plugins, I think there should exist an optional boolean option (a 
little redundant, I know) named 'enabled', so that a plugin can be disabled by 
default (enabled: false) and not have its init method called for each chart on 
the page, saving load time and possibly some execution time when certain events 
are fired.

Here's my hacked version of initPlugins that skips over any disabled plugins:

       function initPlugins() {
            for (var i = 0; i < plugins.length; ++i) {
                var p = plugins[i], name = p.name;
                if (p.options)
                    $.extend(true, options, p.options);

                if (options[name] && typeof options[name].enabled != 'undefined' && 
                    (!options_[name] || typeof options_[name].enabled == 'undefined' || 
                     options_[name].enabled !== true))
                    continue;

                p.init(plot);

            }
        }

If 'enabled' is made into a default plugin option (set to true), then that 
conditional can be a lot shorter. I did it this way so that I didn't have to 
add a disabled option (enabled:false) to all but one of my individual chart 
option objects.

Original issue reported on code.google.com by m...@webxl.net on 4 Aug 2011 at 10:40

GoogleCodeExporter commented 8 years ago
I appreciate the thought you've put into this suggestion, but I'm not (yet) 
convinced this is big enough of a problem to fix in this way. It appears to me 
you should fix your plugin to do the right thing and check itself via options 
whether it should register the listener or not - that way the option to disable 
it can also look a bit more natural, that at least appears to have been the 
case for some of the other plugins. I know this means a bit extra code compared 
to handling it generically, something like

if (!plot.getOptions().blahblah.enabled)
    return;

but it does mean extra flexibility and is perhaps also a bit easier to 
understand from the point of view of the writing a plugin.

Original comment by olau%iol...@gtempaccount.com on 9 Aug 2011 at 10:01

GoogleCodeExporter commented 8 years ago
Thanks for the response. I initially tried your suggestion, but the plugin's 
options aren't accessible using plot.getOptions() because you have them merged 
after init is called, so it didn't work: 

         function initPlugins() {
            for (var i = 0; i < plugins.length; ++i) {
                var p = plugins[i];
 --->           p.init(plot);
                if (p.options)
 --->              $.extend(true, options, p.options);
            }
        }

Is there a reason why those are in that order?

Original comment by m...@webxl.net on 9 Aug 2011 at 6:41

GoogleCodeExporter commented 8 years ago

Original comment by dnsch...@gmail.com on 4 Jun 2012 at 2:41