Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#14972 closed defect (worksforme)

[regression] Concatenated built layers load dependencies too aggressively in legacy mode

Reported by: Colin Snover Owned by: Rawld Gill
Priority: undecided Milestone: 1.7.3
Component: Loader Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by Colin Snover)

I’m trying to create a concatenation of built layers that uses flattened NLS bundles. Unfortunately, it seems that there are at least two places in the loader where dependencies are preemptively requested in response to define calls, which means that, though the chain of dependencies, these files are required *in addition to* their constituent files. The locations in the 1.7 branch corresponding to this problem appear to be:

  1. dojo.js:1466 if(has("dojo-sync-loader") && legacyMode && !waiting[mid]){
  2. dojo.js:1667 if(targetModule && !waiting[targetModule.mid]){

Steps to reproduce:

  1. Create standard build (w/ noref flag).
  2. Create a PHP script like this within the release @dojo@ directory for the build:
<?php

header('Content-Type: text/javascript');
readfile('dojo.js.uncompressed.js');
readfile('../dijit/dijit.js.uncompressed.js');
readfile('../dijit/dijit-all.js.uncompressed.js');
readfile('../dojox/grid/DataGrid.js.uncompressed.js');
  1. Create a valid, blank page containing only this script:
<script data-dojo-config="locale: 'en-us', baseUrl: '//cross-domain/path/to/dojo'" src="//cross-domain/path/to/dojo/concat.php?/dojo.js"></script>

Expected: concat.php and dijit-all_en-us.js load.
Actual: those scripts plus 8 additional scripts load.

Change History (8)

comment:1 Changed 7 years ago by Colin Snover

Description: modified (diff)

comment:2 Changed 7 years ago by Colin Snover

Description: modified (diff)

comment:3 Changed 7 years ago by Colin Snover

This code doesn’t make sense to me. Why is injectDependencies called when an “unrequested” define is found? Why does the loader stall with stuff in execQ when this doesn’t happen?

When there’s an unrequested, non-anonymous define call, it seems to me that the loader should be taking that and storing it for later when some code actually requests the module; I can’t fathom why it should be trying to load and call the factory function at this time.

After spending over 10 hours dealing with this, I’m really no closer to understanding how to solve it, which is frustrating and makes me feel bad about the loader code.

comment:4 in reply to:  3 ; Changed 7 years ago by Rawld Gill

Resolution: worksforme
Status: newclosed

Replying to csnover:

This code doesn’t make sense to me. Why is injectDependencies called when an “unrequested” define is found?

The loader assumes that if a define is provided, the user is intending on instantiating that module. This is the case in 99.99% of the time and employing the algorithm of injecting (but not instantiating) dependencies improves performance for all normal users. The users that it affects adversely are making two errors:

  1. They are using the loader outside the AMD spec where the only way a define application ever occurs is consequent--directly or indirectly--to a require application.
  1. They are not using the advanced loader capabilities available with the dojo loader correctly. This is understandable because they are not well/completely documented yet. But, they are that way for a reason, namely, some of the APIs that these advanced features cover are still being negotiated in the amd implementors community and documenting something that is not needed in the normal case will only serve to get people using stuff that is likely to change.

Why does the loader stall with stuff in execQ when this doesn’t happen?

I don't understand that question.

When there’s an unrequested, non-anonymous define call, it seems to me that the loader should be taking that and storing it for later when some code actually requests the module; I can’t fathom why it should be trying to load and call the factory function at this time.

Because we support backcompat, which allows you to write:

<script src="path/to/dojo/cookie.js"><script>

And have the dojo/cookie module magically be instantiated. This was discussed on several occasions in meetings (and maybe on the list...can't remember).

After spending over 10 hours dealing with this, I’m really no closer to understanding how to solve it, which is frustrating and makes me feel bad about the loader code.

I'm sorry you are frustrated. The number of possible operating modes between sync and async, with legacy dojo.provide/require/loadInit/i18n/text modules, plugins, and xdomain mode is staggering. And the code is further complicated by the need to keep it very small. Until recently, no competing loader came close to being as small as dojo's (note: others are working hard to compete so I don't know if this is still true). Pure AMD mode is quite simple. The rest does get very complex and solving corner case problems is not easy...even for me sometimes.

Clearly, this is not optimal: optimal would be just AMD and nothing else. But that is not an option for 1.x.

comment:5 Changed 7 years ago by Rawld Gill

I duplicated your test case (which was nicely reduced btw). The loader is behaving as designed.

The nls bundles are being demanded because root nls bundles are loaded and the root is indicating that an en bundle is available.

The other modules (e.g.), dnd/Autosource are related to #14819

comment:6 in reply to:  4 ; Changed 7 years ago by Colin Snover

Replying to rcgill:

Why does the loader stall with stuff in execQ when this doesn’t happen?

I don't understand that question.

When I remove the injectDependencies call around dojo.js:1667 (so that it’s just a defineModule call), the module that was defined-without-a-request gets stuck in execQ when something actually wants to use it later.

When there’s an unrequested, non-anonymous define call, it seems to me that the loader should be taking that and storing it for later when some code actually requests the module; I can’t fathom why it should be trying to load and call the factory function at this time.

Because we support backcompat, which allows you to write:

<script src="path/to/dojo/cookie.js"><script>

And have the dojo/cookie module magically be instantiated. This was discussed on several occasions in meetings (and maybe on the list...can't remember).

I understand that that’s the purpose of the *first* block of code I referenced above, but what about the second? Same thing?

After spending over 10 hours dealing with this, I’m really no closer to understanding how to solve it, which is frustrating and makes me feel bad about the loader code.

I'm sorry you are frustrated. The number of possible operating modes between sync and async, with legacy dojo.provide/require/loadInit/i18n/text modules, plugins, and xdomain mode is staggering. And the code is further complicated by the need to keep it very small. Until recently, no competing loader came close to being as small as dojo's (note: others are working hard to compete so I don't know if this is still true). Pure AMD mode is quite simple. The rest does get very complex and solving corner case problems is not easy...even for me sometimes.

I understand. This is certainly not trivial code, though I do have concerns that the drive to be extremely small has led to some maintainability issues and I hope that we can find a friendlier equilibrium somehow since it seems like you’re the only one that is really not totally in the dark. :)

comment:7 in reply to:  6 Changed 7 years ago by Rawld Gill

Replying to csnover:

Replying to rcgill:

Why does the loader stall with stuff in execQ when this doesn’t happen?

I don't understand that question.

When I remove the injectDependencies call around dojo.js:1667 (so that it’s just a defineModule call), the module that was defined-without-a-request gets stuck in execQ when something actually wants to use it later.

Well, if you remove random parts of the code, it probably won't ever work.

Because we support backcompat, which allows you to write:

<script src="path/to/dojo/cookie.js"><script>

And have the dojo/cookie module magically be instantiated. This was discussed on several occasions in meetings (and maybe on the list...can't remember).

I understand that that’s the purpose of the *first* block of code I referenced above, but what about the second? Same thing?

The loader makes no guarantee about how to behave when resented a rollup of several modules. This would add yet more complexity (complexity that already irritates both me and you).

I understand. This is certainly not trivial code, though I do have concerns that the drive to be extremely small has led to some maintainability issues and I hope that we can find a friendlier equilibrium somehow since it seems like you’re the only one that is really not totally in the dark. :)

Patches welcome.

comment:8 Changed 7 years ago by Colin Snover

My deepest apologies on this; there was some testing that I did earlier in the week where I swore the 1.6 loader was acting differently, but looking at it again, it clearly is not—and could not have been doing what I thought it was doing. The real bug is in ticket #14989, which I created and attached a patch for. I’d still like to figure out a way to reduce some of the layers of indirection that are in the loader at some point to try to avoid anyone else ending up where I was.

Cheers,

Note: See TracTickets for help on using tickets.