Opened 7 years ago

Closed 4 years ago

#16373 closed defect (patchwelcome)

dojox.mobile.lazyLoadUtils does not wait for requirements with async loader

Reported by: Philippe May Owned by: Patrick Ruzand
Priority: undecided Milestone: 1.13
Component: DojoX Mobile Version: 1.8.1
Keywords: Cc:
Blocked By: Blocking:

Description

With the async loader and widget's parseOnLoad set to true, the lazyLoadUtils.instantiateLazyWidgets function branches to sync's style dojo.require.

Consequently, the parser fails on the content as the widget's requirements are not fulfilled.

Attachments (1)

lazyLoadUtils_async.diff (389 bytes) - added by Philippe May 7 years ago.

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by Philippe May

Attachment: lazyLoadUtils_async.diff added

comment:1 Changed 7 years ago by Adrian Vasiliu

I do see here the fact that instantiateLazyWidgets uses the synchronous dojo.require even if async:true, but this holds only if the user explicitly requires the legacy "dojo/_base/loader", which doesn't make sense if the user wants async loading, and I don't see why the parser would fail in this case. Do you have a test case or sample which reproduces such a failure? Independently on that, we will check the rationale of the current code.

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:2 Changed 7 years ago by Philippe May

Thanks Adrian for pointing to "dojo/_base/loader". Without loading that module, dojo.require is not defined, so lazyLoadUtils works as expected.

The thing is: i didn't require "dojo/_base/loader" explicitly, but dojo-sync-loader=1 is a part of the defaultConfig in dojo.js. I had to set it explicitly falsy (along with async to truly) to fix the problem.

So either:

  • I missed some obvious point somewhere ;)
  • Dojo base should add some "protection" (at least in development) preventing getting both AMD-async and the legacy loader. This can be another ticket
  • Change the code in lazyLoadUtils, to check if we have async rather than legacy, like in the patch submitted
  • Don't bother at this point, wait for 2.0 and hope all the need for handling legacy will be dropped (?)

Why the parser failed in my case: i think dojo.require was working in async mode, somehow, so the parser was called before the requirements were loaded. I didn't dig in too much details though.

comment:3 Changed 7 years ago by Adrian Vasiliu

Ok, thanks for the details. Some answers:

"i think dojo.require was working in async mode, somehow" - AFAIK dojo.require is always the legacy loader, thus is always sync, not async. But I may be miss something too ;-)

"Dojo base should add some "protection" (at least in development) preventing getting both AMD-async and the legacy loader." - I think the point is that an app may mix old legacy code and new AMD code, so it may need to mix both loaders for loading different pieces of code.

"Change the code in lazyLoadUtils, to check if we have async rather than legacy, like in the patch submitted" - I think there's no reason that we use the legacy loader directly just because the config flag is sync. The new loader handles the sync case by itself. We currently use the legacy loader directly if it has been explicitly required, but we may get rid of that.

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:4 Changed 7 years ago by Philippe May

Thanks again Adrian.

It seems that there are indeed some cases where the legacy loader works in async mode, as documented in the description of dojo.require in dojo/_base/loader.js. It is mentioned: cross domain loading (which is not my case), and other cases which are not clear to me, including a syntax like dojo["require"], and coincidently that's what is used in lazyLoadUtils. That's getting beyond my understanding of Javascripting, btw :)

Last edited 7 years ago by Philippe May (previous) (diff)

comment:5 Changed 7 years ago by Adrian Vasiliu

Right, the legacy loader is not always in sync mode, which may be a even stronger reason for simply removing the special treatment of the if(dojo.require) case in lazyLoadUtils.instantiateLazyWidgets(), as mentioned in a previous comment. When we'll have a bit of time, we'll further dig into it. That said, if I understand well, this issue is not blocking you (anymore), right? or is it? (just to know how urgent this is for you).

Last edited 7 years ago by Adrian Vasiliu (previous) (diff)

comment:6 Changed 7 years ago by Philippe May

Exactly: that's absolutely not blocking for me after your first comment, so probably you can keep this as low priority.

comment:7 Changed 6 years ago by Patrick Ruzand

Owner: changed from Eric Durocher to Patrick Ruzand
Status: newassigned

comment:8 Changed 4 years ago by dylan

Milestone: tbd1.12
Resolution: patchwelcome
Status: assignedclosed

Given that no one has shown interest in creating a patch in the past 2+ years, I'm closing this as patchwelcome.

Note: See TracTickets for help on using tickets.