Opened 10 years ago

Closed 10 years ago

#10612 closed defect (fixed)

dijit._Templated widget does not declare _attachPoints

Reported by: Daniel Stefaniuk Owned by: bill
Priority: high Milestone: 1.4.1
Component: Dijit Version: 1.4.0
Keywords: templated Cc:
Blocked By: Blocking:

Description

The property _attachPoints is not declared by the dijit._Templated "class".

The first occurrence of this property can be found in the body of buildRendering method where it is declared as an empty array. This causes the problem if a new "class" overrides buildRendering method.

Please refer to the ticket #10526.

Change History (6)

comment:1 Changed 10 years ago by bill

Resolution: invalid
Status: newclosed

Hi danstefan,

I saw your comment in the other ticket. It's not wise to declare arrays in a prototype as you suggest:

dojo.declare(foo, [], {
    myArray: []
});

The problem is that all instances of foo and all subclasses of foo will share the same array.

Let me know if there's something I missed.

I'm not sure what the issue is with DTL but I don't see a problem with _Templated at this point. The only references I see to this._attachPoints[] are in buildRendering() and destroyRendering(). If a subclass overrides buildRendering() without overriding destroyRendering() then it would need to create that array.

comment:2 in reply to:  1 ; Changed 10 years ago by Daniel Stefaniuk

Resolution: invalid
Status: closedreopened

Replying to bill:

Hi danstefan,

I saw your comment in the other ticket. It's not wise to declare arrays in a prototype as you suggest:

dojo.declare(foo, [], {
    myArray: []
});

The problem is that all instances of foo and all subclasses of foo will share the same array.

Let me know if there's something I missed.

I'm not sure what the issue is with DTL but I don't see a problem with _Templated at this point. The only references I see to this._attachPoints[] are in buildRendering() and destroyRendering(). If a subclass overrides buildRendering() without overriding destroyRendering() then it would need to create that array.

I'm sorry bill I decided to reopen the ticket and explain it more clearly. This is a design issue. In the proper OO language

  • _attachPoints is a protected instance variable (used in buildRendering, _attachTemplateNodes and destroyRendering)
  • buildRendering is a public method

Assuming this it has to be declared somewhere. You cannot just start using this in one of methods. It is a bad practice.

In case of dojox.dtl._Template only buildRendering method is overridden however _attachTemplateNodes and destroyRendering are inherited and invoked from DTL's code. buildRendering in dojox.dtl._Template isn't a place to "redeclare" it...

You are right about arrays in a prototype. I think a good patch has been submitted by ID, see #10525 ticket.

comment:3 in reply to:  2 Changed 10 years ago by Daniel Stefaniuk

Replying to danstefan:

Valid ticket number is #10526, sorry.

comment:4 Changed 10 years ago by ID

On a side note I do this in my code:

dojo.declare(foo, [], {
    // Some comment here that explain what is the purpose of this field
    arrWhatever: null,

    constructor: function() {
        this.arrWhatever = [];
    }
});

So I know that arrWhatever is a member of foo ( it shows up in my IDE ) and it's properly initialized.

comment:5 Changed 10 years ago by bill

Owner: set to bill
Status: reopenednew

Ah I see, declaring it in the constructor makes sense since subclasses can't override the constructor (only add additional commands). OK, I'll move the creation of the array from buildRendering() to constructor().

ID, about having:

arrWhatever: null

in your prototype, that's interesting too, it's good that it makes your IDE work, although of course the downside is the added code size.

comment:6 Changed 10 years ago by bill

Resolution: fixed
Status: newclosed

(In [21124]) Declare _attachPoints in constructor() rather than buildRendering() so that DTL, which overrides buildRendering(), works again. Problem started in [20248] (refs #9966).

Thanks to ID and danstefan for the fix. Fixes #10612, #10526 !strict.

Note: See TracTickets for help on using tickets.