Opened 12 years ago

Closed 7 years ago

#5144 closed enhancement (invalid)

Have a short function wrapper for debugAtAllCosts

Reported by: Neil Roberts Owned by: Neil Roberts
Priority: low Milestone: future
Component: Core Version: 1.0
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Adam Peller)

Have something like:

dojo.addOnIfDebug = function(fn){

if(djConfig.debugAtAllCosts){

dojo.addOnLoad(fn);

}else{

fn();

}

}

Change History (17)

comment:1 Changed 12 years ago by Neil Roberts

Type: defectenhancement

comment:2 Changed 12 years ago by James Burke

You can just use dojo.addOnLoad() always, and that will work in the debugAtAllCosts case, and in the "normal" loader case. In the normal loader case, if all module resources are loaded and the domcontentloaded trigger has fired, it will execute the function immediately.

Or are you finding that is not sufficient?

comment:3 Changed 12 years ago by Neil Roberts

You just answered you own question. "if all module resources are loaded and the domcontentloaded trigger has fired". I'm proposing something in which only one of those ifs is true.

comment:4 Changed 12 years ago by Neil Roberts

Milestone: 1.0.2

comment:5 Changed 12 years ago by James Burke

It seems you are concerned only with the normal dojo loader, where you are interested when all the modules load (which is easy, since the dojo.require XHR calls are synchronous). However, in an xdomain loader case, the proposed dojo.addOnIfDebug() function could execute before all modules are loaded. In that case, this new function will cause problems in the xd case.

I wonder if it is easier to always have the developer assume that dojo.require calls may not be completed right away (assume async), so if they do switch to debugAtAllCosts or an xd build, there will be fewer surprises. I would prefer that approach, but it is not something that has been formally adopted by dojo. If we go with that approach, then I think dojo.addOnLoad() is sufficient as it is today.

comment:6 Changed 12 years ago by Adam Peller

what's the justification for putting this in 1.0.2?

comment:7 Changed 12 years ago by Neil Roberts

I only put some example code up, no reason it couldn't be tweaked for other situations.

My personal experience with wishing this was here is that I wanted to declare a widget, but as part of that declaration, I needed to use a variable from one of the files in a dojo.require. I don't feel that this type of code is appropriately placed in an addOnLoad.

The justification for 1.0.2 is because it's really easy to do and would help make debugAtAllCosts a little less confusing for those that want to use it.

comment:8 Changed 12 years ago by Neil Roberts

The alternative might be to use the script type="dojo/etc" syntax and treat that block of code as a standard required file.

comment:9 Changed 12 years ago by Adam Peller

Milestone: 1.0.21.1

This sounds like something we should really work out on trunk, and not a critical 1.0 fix, but we can revisit this after it lands if you want.

comment:10 Changed 12 years ago by James Burke

I think the awkward thing about using dojo.addOnLoad() is that it runs after widget parsing, and it could run after other code that does a dojo.addOnLoad(), but that other code might be dependent on the object you are trying to define with the first addOnLoad() call.

Seems like a new API call like dojo.addOnResourcesLoaded() might be what you are looking for. I think the script type="dojo/etc" will probably have the same problem as dojo.addOnLoad() since it will run after page load, unless we put a special exception in the parser code.

It seems like the dojo.addOnResourcesLoaded() approach would work better, and also work in other xdomain cases where you might want to do a delayed set of dojo.require calls followed by some code that depended on those require calls to be completed before proceeding. All of that normally happens in pure JS code.

I'm still on the fence about this though. The downside with using dojo.addOnLoad() normally means you have to set djConfig.parseOnLoad = false, then call dojo.parser.parse() manually after you do the other stuff in your dojo.addOnLoad() call.

We have to weigh that awkwardness with introducing a new API call, and explaining more of the dojo internals that give rise to this API call.

Unfortunately, I think this issue comes up more than I would like, since there are book/tutorial examples that use the CDN build. Those examples are usually "one file" examples that can have widgets, so the need to call dojo.parser.parse() in your addOnLoad() call shows up more to the end user.

So it is probably worth introducing a new API function, so that the parseOnLoad: true approach can be used for those examples. But that makes it harder to talk about Dojo 1.0 and 1.1 in the same example: you will need two different examples. Ugh. Maybe something to consider more for 2.0.

comment:11 Changed 12 years ago by Neil Roberts

Yeah, what you describes is all I'm looking for The thing is that I want to use debugAtAllCosts mostly while I'm developing code, and I don't want to have to move everything to separate files before I know exactly how I'm going to use them.

comment:12 Changed 12 years ago by dylan

Milestone: 1.11.2
Priority: highnormal

No patch yet, and not a defect critical for 1.1, so pushing to 1.2

comment:13 Changed 11 years ago by Adam Peller

Description: modified (diff)
Milestone: 1.2future

comment:14 Changed 11 years ago by James Burke

Owner: changed from alex to James Burke

comment:15 Changed 8 years ago by ben hockey

Owner: changed from James Burke to Neil Roberts
Status: newpending

is this ticket still valid? if there's no response within 14 days it will automatically close

comment:16 Changed 8 years ago by ben hockey

Priority: highlow

comment:17 Changed 7 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Note: See TracTickets for help on using tickets.