Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 20 #21

Merged
merged 1 commit into from
Feb 15, 2012
Merged

Issue 20 #21

merged 1 commit into from
Feb 15, 2012

Conversation

jeremyg484
Copy link
Member

Includes:

  • an updated test for the Dojo pubsub plugin that illustrates the problem
  • a fix

@jeremyg484
Copy link
Member Author

My mistake...didn't realize my pull request would create a new issue.

@briancavalier
Copy link
Member

No worries, Jeremy, I appreciate the pull request, with unit tests! What a n00b mistake on my part :) Sorry for any headaches this caused you. I'll take a look at it in the morning and hopefully get it merged in.

briancavalier added a commit that referenced this pull request Feb 15, 2012
@briancavalier briancavalier merged commit 3eb1513 into cujojs:dev-080 Feb 15, 2012
briancavalier added a commit that referenced this pull request Feb 15, 2012
… tests. Dojo seems to be causing long load times with curl and requirejs seems to fail to load parts of dojo. Unit tests pass in curl, but fail in requirejs due to dojo relying on require.ready, which appears to be specific to the dojo loader now.
@jeremyg484
Copy link
Member Author

Thanks, Brian. Out of curiosity, are you using seeing the slow loading problem with Dojo 1.6 or 1.7? I originally made the fix in my project that uses 1.7, but did my testing for the pull request against 1.6 and didn't notice the issue.

@neonstalwart
Copy link

@briancavalier i didn't dig very deep but it's possible that dojo is loading in sync mode due to the loading it via another loader. this causes it to load all of dojo base via synchronous xhr which is likely going to take a while. if that's the issue, the fix would be to set the global dojoConfig.async to something truthy before loading dojo.

@briancavalier
Copy link
Member

@neonstalwart Thanks! I'll give that a try. Will that option affect sync/async loading even if dojo is being loaded by another loader, like curl or requirejs?

@jeremyg484 I'm seeing it with dojo 1.6.1. I can try 1.7 as well and see if it's similar.

@neonstalwart
Copy link

@briancavalier sorry... i'm misleading you and confusing myself. let me start over - ignore my previous comment about dojoConfig.async.

i see the "dojo" dependency in wire/wire/dojo/pubsub and presumably that's configured somehow to resolve to "dojo/main". if that's the case then by requiring "dojo" you'll get all of dojo base (way more than you need) as dependencies to be loaded (presumably asynchronously) via whichever loader (curl, requirejs, dojo or whatever) is loading "dojo/main".

a better dependency in this case would be "dojo/topic". this exports a publish and subscribe and is how dojo.publish and dojo.subscribe are now implemented. it would also be a significantly smaller size when built.

btw, all of this is only relevant to dojo 1.7 and personally i wouldn't even try with 1.6 but if you need to support that then good luck but that's almost like trying to support 2 different libraries rather than 2 versions of the same library since, with respect to AMD, modules changed significantly from 1.6 to 1.7

@briancavalier
Copy link
Member

Ah, thanks for the detailed explanation, Ben, that makes sense. When I first wrote wire's dojo pubsub plugins, dojo 1.6 was the official release so that's what I coded against. I'd love to update the plugin to take advantage of dojo/topic and the smaller load, but I know there are some wire users still on dojo 1.6. Wire 0.8 might be a reasonable point to start insisting on dojo 1.7, though. I'll try updating wire/dojo/pubsub to work with dojo 1.7, and see how that goes.

@unscriptable
Copy link
Member

@neonstalwart just to confirm: dojoConfig.async is not needed when loading with a non-dojo loader, correct? That was my interpretation or the code and code comments. :)

@neonstalwart
Copy link

@unscriptable right - dojoConfig.async only matters to the dojo loader (dojo/dojo.js).

however, i don't know what kind of mileage you'll get by trying to load dojo/dijit without using dojo's loader - nobody that i know of has tried to find all the dark corners you might come across. hopefully its not too ugly but feel free to contact me if you've got questions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants