Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#13255 closed defect (fixed)

xdomain mode and build are not 100% backcompat

Reported by: Rawld Gill Owned by: Rawld Gill
Priority: high Milestone: 1.7
Component: Loader Version: 1.7.0b1
Keywords: Cc:
Blocked By: Blocking:

Description

Consider the following legacy module:

dojo.provide("myApp.myModule");

// Section A

dojo.require("myApp.myOtherModule);

// Section B

As of 1.7b1, this is transformed (both in a build and during on-the-fly xdomain loading) to:

define("myApp/myModule", ["dojo", "dijit", "dojox", "myApp/myOtherModule"], function(dojo, dijit, dojox){

// Section A
// Section B
});

This yields the code path:

  1. myApp/myOtherModule
  2. Section A
  3. Section B

In most cases, this is OK since Section A is empty. However, when Section A is not empty, this is different than version v1.6- which would ensure the code path:

  1. Section A
  2. myApp/myOtherModule
  3. Section B

Both the loader and the builder should be improved to ensure the correct backcompat code path.

Since the alternative (v1.7b1) code path allows for some optimizations (for example, the loader doesn't have to guarantee an entire dependency tree is downloaded before starting to execute modules), it should be retained as an option.

Change History (10)

comment:1 Changed 8 years ago by Rawld Gill

Solving this issue by introducing the "dojo/require!" plugin, which does not call loaded on it's resource until the resource and its entire dependency tree have been downloaded (but not executed). This allows the example given in the report description above to be transformed (both in a build and during on-the-fly xdomain loading) to:

define(
  "myApp/myModule", 
  ["dojo", "dijit", "dojox", "dojo/require!/myApp/myOtherModule",
  function(dojo, dijit, dojox){

  // Section A

  dojo.require("myApp.myOtherModule);

  // Section B
});

Notice that myApp/myOtherModule is not executed until the factory function for myApp/myModule demands it via dojo.require. This causes the v1.6- execution path of

  1. Section A
  2. myApp/myOtherModule
  3. Section B

It is impossible to express dojo/require! as a normal AMD plugin, external to the loader; therefore, it is expressed as an special, built-in plugin within the dojo loader. Since this machinery is only necessary for maintaining backcompat, the implementation is bracketed by has("dojo-sync-loader") just as all backcompat (nonstandard AMD) machinery contained in the loader.

Though the plugin solution is not strictly required (the problem could be solved by internal loader machinery only), it helps control complexity within the build application by limiting the span of backcompat hacks to solve this issue.

comment:2 Changed 8 years ago by Rawld Gill

See also #13201. dojo/require! provides a method to solve that problem. Note however, the solutions have significant differences.

Difference 1: With dojo/provide!, order of execution is not backcompat (it is the not-backcompat order reported in the summary of this ticket). This may or may not be an issue depending on the module being loaded (it will not be an issue for most legacy modules).

Difference 2: With dojo/provide!, the first module in a legacy tree must be explicitly marked by dojo/provide!. No action is required by the programmer with dojo/require! other than using the loader in a legacy loading mode (which may be asynchronous either temporarily during xdomain loading or permanently by setting the configuration variable async to "async").

Difference 3: dojo/provide! favors script injection over XHR. The dojo/provide! resource is downloaded via async XHR, then any module within that module's dependency vector (after downloading via async XHR) that includes a dojo.provide application is assumed to be another dojo/provide!'d module; modules that do not contain a dojo.provide application are assumed to be AMD modules and script injected (which results in two demands for the same module, but with a reasonable browser cache this shouldn't cause too much trouble). Again, in most cases this will work fine. However, there are legacy modules that do not contain dojo.provide (that's the purpose of the second parameter to dojo.require). In contrast, dojo/require! downloads every not-xdomain resource (AMD or legacy) via XHR. If the loader is in "sync" legacy mode, then a sync XHR is used; otherwise an async XHR is used.

Difference 4: dojo/provide! does not account for dojo.loadInit, dojo.requireIf, dojo.platformRequire, dojo.requireLocalization. Of course these could be added.

comment:3 Changed 8 years ago by Rawld Gill

Status: newassigned

Solving several backcompat issues during the break-in period with the new loader has resulted some cruft accumulating in the loader. The following issues are also cleaned up in this ticket:

  1. Remove the require.error API which has proven not useful; replace with the signal "error". The loader includes the ability to signal listeners since the addition of dojo/domReady. With this feature, it is nearly free to replace require.error with the signal "error", which allows clients to connect and listen for errors.
  1. With tracing turned on, the loader uses a Javascript implementation of symbols to help with debugging. Bryan Forbes pointed out that some browsers don't display symbols nicely and strings would be better. Though using string for identity comparison is not optimal, it is fine during debugging; therefore replace symbols with strings when tracing is turned on.
  1. The loader optionally (depending on has("dojo-loader-catches")) protects critical areas with try-catch. This made debugging difficult. Therefore, remove try-catch and replace with try-finally.
  1. The internal loader function "on" was originally intended to be made public and therefore included additional features not needed by the loader. This function will never be public; therefore, simplify it accordingly.

comment:4 Changed 8 years ago by Rawld Gill

(In [25543]) removed old loader tests not being used; refs #13255

comment:5 Changed 8 years ago by ben hockey

in 25542 http://bugs.dojotoolkit.org/browser/dojo/trunk/i18n.js?rev=25542#L157 references require.isXdUrl which is not available when dojo is not the loader.

comment:6 Changed 8 years ago by Rawld Gill

The enhanced dynamic detection and processing of the v1.x sync API in the loader is being applied to built modules which causes those modules to express semantics not intended. The loader/build system need to work together to understand when a module is built and not apply text processing in those cases.

comment:7 Changed 8 years ago by Rawld Gill

(In [25575]) added feature to flag built modules as such to the dojo synchronous loader; refs #13255; !strict

comment:8 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

(In [25768]) moved legacy loader API out of loader into separate module; completed dojo/require and associated dojo/loadInit plugins; removed remaining bdLoad cruft; fixed/added tests; other cleanup (see #13255); fixes #13255; !strict

comment:9 Changed 8 years ago by Rawld Gill

In [25768] several inter-dependent enhancements were committed as follows:

The significant bulk of the change set addresses these issues:

  1. The legacy loader API was moved from the loader (dojo/dojo.js) to the legacy backcompat loader module (dojo/_base/loader.js). This decreases the code size/complexity in dojo.js. Also, some duplicate XHR handling was removed from the legacy loader impl and replaced with dojo.xhr; this was made possible owing to the reorg that took the legacy loader impl into a normal module.
  1. The dojo/require and complementary dojo/loadInit plugins were completed. These reside in dojo/_base/loader; the modules dojo/require and dojo/loadInit simply point to routines published by dojo/_base/loader. These plugins allow for 100% code path compatibility in the various combinations of AMD/legacy modules, unbuilt/built modules, not-xdomain/xdomain, and async/sync/legacy async/xdomain modes of operation. See loader tutorial on dojo campus for details.
  1. The builder was modified to take advantage of the dojo/require and dojo/loadInit design so that it can transform legacy modules to AMD modules, and further those modules will faithfully follow the 1.6- code path.
  1. 2 and 3 above also allow for proper handling of dojo.requireIf and dojo.loadInit; early plans for 1.7 were to ignore this problem; however, the new plugins allowed a nice solution; accordingly, it was implemented.
  1. The transforms that take place on-the-fly (in the xdomain loader) and in the build app have been improved to optionally rely less on regular expressions. This enhancement has a cost in the builder that transformation can be more noisy, reporting many false errors. Accordingly, error reporting was also improved to control the amount and quality of feedback.
  1. Classifying modules as either AMD or legacy modules is important to achieve best build app performance. Previous attempts to accomplish this classification in the package.json file were abandoned owing to the limitations of JSON. Instead, package.json now includes a single, optional property "dojoBuild" that points to a profile file that instructs the build application with default values for a particular package. package.json and default profile files were updated/added in dojo, dijit, dojox, and demos.
  1. A final few details were worked out regarding the rescoping (AKA multi-version support / relocating). A significant test/example was added, demonstrating both dev and built versions. See dojo/tests/_base/loader/coolio.
  1. Extensive testing was done with xdomain loading, paying particular attention to following v1.6- codepaths. See dojo/tests/_base/loader/xdomain
  1. Many loader tests were added and/or enhanced. Minor inconsistencies in non-loader tests were fixed. Tests can now be ignored, copied, or built with the build app. All versions appear to be working correctly.
  1. Some cruft from the run-in period as well as some left from bdLoad was removed. In particular the old bdLoad "package-qualified name" concept was dropped, and this resulted in a global change to the way modules are addressed in the loader and build app (the property "mid" is used rather than "pqn"; the properties "pqn" and "path" were dropped); this minor change causes a good portion of the changeset. Also, several experimental has feature switches that proved not useful were removed.

comment:10 Changed 8 years ago by liucougar

(In [25803]) refs #13255: remove reference to dijit in dojo profile

Note: See TracTickets for help on using tickets.