launchpad-dev team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

> So no, don't do:
> 
>  * global event publishing
>  * YUI.Env as a global namespace
> 
> This last is especially ugly.  It defeats the whole point of the js
> sandbox when there's no need for it.
> 
> How do we fix these issues then?
> 
> Everything should live in a proper YUI module now.  Any module that
> depends on functionality in another should be sure that dependent
> module is included via the "use" block.  If something needs to be
> shared across modules, then that functionality should be broken out
> into its own module.  I'm not able to offer more than this without
> digging deeper into the specifics of the failing code, but I am highly
> confident that we should avoid the two suggestions above at all costs.
> 
> And apologies if I'm being too harsh here.  I've had a grouchy Monday,
> and I'm just trying to make sure I'm clear that this is not a pattern
> we want to start in Launchpad.  If we need to talk about design
> choices or about how to best refactor the spots causing us issues,
> then I can stay later one day this week to catch up with you, Ian,
> about it.

Here's a specific use case where I can't see right now a solution which
does not require global events, especially if we want to implement a
solution with the correct IOC and minimal coupling.

In client.js we have a model cache implementation. The namespace is
lp.client and there's an update_cache() method. When update_cache() is
invoked, we need to let interested parties know what changes have been
made. This is a classic publish-subscribe pattern: the publisher (the
cache object) has no knowledge of what consumes it's update
notifications (the subscribers being bespoke bits of code throughout the
app). And nor should it, it's re-usable infrastructure etc etc.

A common pub-sub implementation is to use broadcast events as the
transport. Message queues etc can also be used. But, this work was
originally done using YUI's event system. And it worked a treat before
we switched away from a single, shared YUI instance. So the cache simply
calls:

Y.fire('lp:context:foobar:changed', {new/old foobardata});

It has no knowledge of who is interested in these updates. On the
subscriber side, there exists on various pages code like:

YUI().use('io-base', 'lp.ui', 'lp.client', function(Y) {

    Y.on('lp:context:deb_version_template:changed', function(e) {
        Y.lp.ui.update_field('#debian-version dd', e.new_value);
    });
    Y.on('lp:context:base_branch_link:changed', function(e) {
        Y.lp.ui.update_field('#base-branch dd', e.new_value_html);
    });
});

So the page listens to model updates and does stuff when they are
received. But the code now lives in it's own sandboxed instance, and
events fired from the cache are no longer received.

Rick provided some sample code which worked across instances
http://paste.mitechie.com/show/523/, but in that example, the code doing
Y.fire had to know about the YUI module which was to receive the events.
We must not allow that pattern to be used here. The cache must not know
about how it is used or what uses it or be forced to YUI import the
subscriber code via YUI.use().

At the moment, I cannot see a solution that:
- avoids the use of global events
AND
- does not introduce unnecessary coupling
- does not mess up dependencies
- does not force lower layer infrastructure to know about bespoke app code
- does not force us to migrate immediately away from our legacy codebase

It could be that I've looked at this too long and my brain cannot see an
obvious solution. Now's the chance to show me how dumb I am. Go ahead,
you know you want to.....