Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#14010 closed defect (fixed)

build assumes layer.name property points to an existing source module

Reported by: Rawld Gill Owned by: Rawld Gill
Priority: high Milestone: 1.7
Component: BuildSystem Version: 1.7.0b1
Keywords: Cc: ykami, ben hockey, Chris Mitchell
Blocked By: Blocking:

Description

Usually, this error wont matter as the filename given for a layer module will, in fact, point to an existing source module. However, if the output tree is being relocated (e.g., a user tree that is not a sibling of dojo is being made a sibling of dojo) then the build system fails to resolve the layer into a proper module.

Change History (24)

comment:1 Changed 8 years ago by Rawld Gill

Status: newassigned

comment:2 Changed 8 years ago by Rawld Gill

The v1.6- build system "flattens" the module structure, no matter how it is arranged on input, into a set of sibling top-level modules (dojo, dijit, dojox, demos, myStuff, yourStuff, etc.). The layer.name property is just a filename. Theoretically, it could be placed anywhere, but in practice, it's always placed somewhere in this flattened forest of module trees by giving a name like "../myTopLevelModule/someModule.js". Therefore, the intended module name can be deduced by chopping off the "../" prefix and ".js" suffix. Again, in theory, this won't work 100% of the time, but we don't have any examples of it not working. This technique also works for layerDependencies. Therefore, we use this algorithm to transform v1.6 layers into v1.7 AMD module destinations.

comment:3 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [26707]:

changed v1.6 profile transform algorithm that decodes layers; fixes #14010; !strict

comment:4 Changed 8 years ago by ykami

Resolution: fixed
Status: closedreopened

The mobile build size doubled due to this change. Profiles are dojo/util/buildscripts/profiles/mobile*.js. It looks dojo/dojo.js is being picked up unexpectedly. Please reconsider this change.

comment:5 Changed 8 years ago by ykami

Cc: ykami added

comment:6 Changed 8 years ago by ben hockey

Cc: ben hockey added

comment:7 Changed 8 years ago by Chris Mitchell

Priority: normalhigh

bumping priority to high to get it on the hot list.

comment:8 Changed 8 years ago by Chris Mitchell

Cc: Chris Mitchell added

comment:9 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

In [26742]:

fixed wrong customBase and version number in stock profiles; fixes #14010

comment:10 Changed 8 years ago by Rawld Gill

The customeBase parameter is required when building a custom base (this is not a change in 1.7). This typo was fixed in [26742]; please rerun the build to confirm you are getting what you expect.

comment:11 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: closedreopened

The loader is being improperly appended to dojo.js module.

comment:12 Changed 8 years ago by Rawld Gill

Resolution: fixed
Status: reopenedclosed

In [26744]:

fixed dojo/dojo layer being written twice/as a separate boot layer; further enhanced backcompat jankies for layerDependencies; \!strict; refs #14020; fixes #14010

comment:13 Changed 8 years ago by ykami

In [26747]:

Refs #14010 !strict. Reverted the previous change. This is not a customBase example.

comment:14 Changed 8 years ago by ykami

Verified that the problem was fixed. But I reverted the change you made to mobile.profile.js. Then, everything seems back to normal.

comment:15 in reply to:  10 Changed 8 years ago by ben hockey

Replying to rcgill:

The customeBase parameter is required when building a custom base (this is not a change in 1.7).

fyi, you actually never needed the customBase parameter when adding modules to dojo.js (see baseplus and layers profiles as points of reference) - you only needed it for when you wanted a subset of base. ie that parameter only applied to anything in dojo._base.

comment:16 Changed 8 years ago by ben hockey

@ykami - i looked into what rawld was talking about with customBase. i had thought that he was saying that customBase needed to be true to include other modules in dojo.js. as i already mentioned, this is not needed for this case but it turns out that this is probably not what he meant. if customBase is not true then all dojo/_base/* modules are included in dojo.js even if no module has a dependency on those modules.

for example, if i have a file (my/Foo.js) that is like this:

define(['dojo/_base/array'], function (array) {
  // ...
]);

and then i have a layer in my profile that has:

{
  name: 'dojo.js',
  dependencies: ['my/Foo']
}

then dojo.js will include all of dojo/_base/* - this undoes all of the hard work that has been done to try and specify granular dependencies.

however, if i change my layer to have customBase: true...

{
  name: 'dojo.js',
  customBase: true,
  dependencies: ['my/Foo']
}

then dojo.js will have the loader and only the base modules needed. i think you might prefer to have customBase: true in your mobile profiles for the dojo.js layer. i just looked at the difference in the raw size of dojo.js before and after using customBase: true in mobile.profile.js:

  • before: 130359 bytes
  • after: 87769 bytes

this is very significant!

comment:17 Changed 8 years ago by ykami

Yes, that has not been changed since customBase was first introduced in dojo-1.2 or so.

comment:18 Changed 8 years ago by ben hockey

ok... i just wanted to make sure that you know you're getting an extra 40k+ in the mobile build even though its not needed.

comment:19 Changed 8 years ago by ykami

mobile-all.profile.js is a minimal build example with the customBase option. The build includes minimum dojo base modules, minimum dijit base modules, and dojox.mobile base modules. mobile.profile.js is an example of creating ordinary dojo.js (to be able to be shared with user application) and dojox.mobile-only layer build. (Thus we intentionally do not use customBase for this sample profile.)
So everything is under control. But thank you for worrying about it. :-)

comment:20 Changed 8 years ago by ykami

On a side-note, customBase is not a magic. The reason why the mobile build size could be reduced with customBase is that we've been putting much effort into developing dojox.mobile with as less dojo/dijit base module dependencies as possible. I don't think customBase has much effect on typical desktop dojo apps, because usually they actually use (=have dependency on) many of the base modules.

comment:21 Changed 8 years ago by ykami

On another side-note, the reason why we do not use customBase in mobile.profile.js is that, if we use it, the output will be:

  1. Ultra lightweight but almost useless dojo.js
  2. dojox.mobile base build with necessary dojo modules that are missing from the above useless dojo.js

There is no point in such layer separation. They should be merged together from the begining. And mobile-all.profile.js is for such a build.

If we do not use customBase in mobile.profile.js, the output will be:

  1. normal dojo base build
  2. dojox.mobile base build without any dojo base modules

Of course the dojo base build (1.) includes modules that are not necessary for dojox.mobile itself, but the assumption is user apps use them. Also, in some scenarios, only the dojox.mobile base build (2.) may be useful since it contains only dojox.mobile modules without dependent dojo modules.

comment:22 Changed 8 years ago by ben hockey

for me the confusion simply came from the names of the profiles. if i see something named "foo" and another thing named "foo-all" i would expect "foo" to be the most optimized version and "foo-all" to be the less optimized choice. i can see what you're doing in both profiles - i was just assuming that the intention was the opposite based on the names.

comment:23 Changed 8 years ago by ykami

I see. But those profiles are supposed to be used from batch files in the dojox/mobile/build folder.

comment:24 Changed 8 years ago by bill

Component: GeneralBuildSystem

Looks like these are builder bugs.

Note: See TracTickets for help on using tickets.