Opened 8 years ago

Closed 8 years ago

#14238 closed defect (fixed)

dojo/fx module adds references to dojo/_base/fx properties onto dojo.fx namespace

Reported by: Kenneth G. Franqueiro Owned by: Bryan Forbes
Priority: high Milestone: 1.7.1
Component: fx Version: 1.7.0
Keywords: fx Cc:
Blocked By: Blocking:

Description (last modified by Kenneth G. Franqueiro)

In Dojo 1.6, the following properties are defined under dojo.fx when the dojo/fx module is loaded:

  • Toggler
  • chain
  • combine
  • slideTo
  • wipeIn
  • wipeOut

However, in 1.7, in addition to these, all of the properties defined by dojo/_base/fx are also present, because the dojo/fx module starts by setting dojo.fx = baseFx. Thus, in 1.7, the dojo.fx namespace ends up looking as follows:

  • Animation
  • Toggler
  • _Line
  • _defaultEasing
  • _fade
  • anim
  • animateProperty
  • chain
  • combine
  • fadeIn
  • fadeOut
  • slideTo
  • wipeIn
  • wipeOut

This can be easily tested with code such as the following:

<!DOCTYPE html>
<html>
  <body>
    <script src="dojo/dojo.js"></script>
    <script>
dojo.require("dojo.fx");
dojo.ready(function(){
  console.log("dojo.fx:", dojo.fx);
});
    </script>
  </body>
</html>

Note that #14239 is likely tangential to this.

Change History (9)

comment:1 Changed 8 years ago by Kenneth G. Franqueiro

Description: modified (diff)

comment:2 Changed 8 years ago by bill

I wouldn't call this a regression. Does it break any existing code?

comment:3 Changed 8 years ago by Kenneth G. Franqueiro

Summary: [regression] dojo/fx module adds references to dojo/_base/fx properties onto dojo.fx namespacedojo/fx module adds references to dojo/_base/fx properties onto dojo.fx namespace

Eh, I see your point. I haven't seen it break any code, and it's unlikely to unless we have code that seriously iterates the members of dojo.fx. But it clearly changes the topography of the dojo.fx namespace in a way that shouldn't be happening, and that people likely shouldn't get used to.

comment:4 Changed 8 years ago by bill

I thought the current behavior might be by design. When 2.0 comes along, since there will be no dojo/_base anymore, it seems likely that all the symbols from dojo/_base/fx and dojo/fx will end up together as the return from a dojo/fx module (a.k.a. dojo.fx).

I guess we could have dojo/fxSimple and dojo/fxAdvanced modules instead.

comment:5 Changed 8 years ago by Bryan Forbes

This is clearly a bad conversion to AMD for a couple of reasons: dojo.fx wasn't ever set to dojo._base.fx and shouldn't be now, and _base functions should not be attached to fx (since they weren't before). Regarding 2.0, the module should be more granular, not less. Putting all effects related functions into one module would defeat that purpose. In 2.0, I forsee the following modules:

  • dojo/Animation
  • dojo/fx
  • dojo/fx/fade
  • dojo/fx/wipe
  • dojo/fx/slide
  • dojo/fx/Toggler

I'll check in the change to dojo/fx (and any subsequent modules that are using the incorrect module return value) to both the 1.7 branch and trunk.

comment:6 Changed 8 years ago by Bryan Forbes

In [27046]:

Fix AMD conversion of dojo/fx and modules using it (refs #14238).

comment:7 Changed 8 years ago by Bryan Forbes

In [27047]:

Fix AMD conversion of dojo/fx. (refs #14238)

comment:8 Changed 8 years ago by Bryan Forbes

In [27048]:

Fix modules incorrectly using dojo/fx. (refs #14238)

comment:9 Changed 8 years ago by Bryan Forbes

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.