Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#15814 closed defect (fixed)

Dojo build appending layer includes as text to the built layer

Reported by: lee Owned by: Rawld Gill
Priority: undecided Milestone: 1.8.1
Component: BuildSystem Version: 1.8.0rc1
Keywords: Cc:
Blocked By: Blocking:

Description

This is actually in 1.8.0.rc2. I am building a single layer and finding that the layer includes from the build profile are appended as text to the end of the file, therefore causing JS errors. I don't know if it's a problem with my build profile, package.json or package profile so I've attached all 3. I've actually removed all module code from the package incase there were errors in the code, therefore it's just an empty directory with the package.json and package profile.

With the files I've attached, this only occurs in the unminified files. However I also found it can effect both if I changed my profile to http://scsys.co.uk:8002/204846


For the files I've attached, the end of releasecode.js.uncompressed.js looks like this:

(function(){
	// must use this.require to make this work in node.js
	var require = this.require;
	// consume the cached dojo layer
	require({cache:{}});
	!require.async && require(["dojo"]);
	require.boot && require.apply(null, require.boot);
})();
dojo/dom

If I include other modules in my layer it just appends them comma seperated e.g. dojo/dojo,dojo/dom

Attachments (4)

build.profile.js (1.1 KB) - added by lee 10 years ago.
build profile
blah.profile.js (138 bytes) - added by lee 10 years ago.
package profile
package.json (130 bytes) - added by lee 10 years ago.
blah package json
15814.patch (877 bytes) - added by lee 10 years ago.
temporary "fix"

Download all attachments as: .zip

Change History (14)

Changed 10 years ago by lee

Attachment: build.profile.js added

build profile

Changed 10 years ago by lee

Attachment: blah.profile.js added

package profile

Changed 10 years ago by lee

Attachment: package.json added

blah package json

comment:1 Changed 10 years ago by lee

And my build source structure looks like:

build.profile.js

  • dojo
  • dijit
  • dojox
  • util
  • blah
    • package.json
    • blah.profile.js

comment:2 Changed 10 years ago by lee

More info is provided in build-report.txt which I'm guessing is where the closure compiler is evaluating the dojo/dojo,dojo/dom text thinking it's a computation

/var/www/dojosrc/dojo-release-1.8.0rc2-src/blahrelease/releasecode.js:16846: WARNING - Suspicious code. The result of the 'div' operator is not being used.
dojo/dojo,dojo/dom
^

/var/www/dojosrc/dojo-release-1.8.0rc2-src/blahrelease/releasecode.js:16846: WARNING - Suspicious code. The result of the 'div' operator is not being used.
dojo/dojo,dojo/dom
          ^

comment:3 Changed 10 years ago by lee

Just to note it's not a regression. I ran the same build with 1.7.3 and still have the same errant text in .uncompressed.js

comment:4 Changed 10 years ago by lee

I did some tinkering and found if I changed the release profile to:

    layers: {
	        "dojo": {
	            include: [               
	                "dojo/dom"
	            ],
	            customBase: 1,
	            boot: 1           
	        }
	    }

It builds the uncompressed layer fine without the extraneous text.

If I change it to

    layers: {
	        "blah": {
	            include: [               
	                "dojo/dom"
	            ],
	            customBase: 1,
	            boot: 1           
	        }
	    }

It'll create a main.js.uncompressed.js with the extraneous text (in the package blah)

There seems to be a dependency on the layer release name and package name I'm not understanding either?

For the moment I'll create a single release layer named "dojo" instead of "blahrelease/releasecode" (see earlier release profile build.profile.js) or "blah" because the layer "dojo" builds fine.

RawId?, can you email me to give me some pointers to help me debug the problem?

comment:5 Changed 10 years ago by lee

A small update as I've spent some time debugging the build.
The problem is definitely before the writeOptimized transform, I also noticed if I set the boot layer to false the dependency text is not appended, however a boot layer is what I want.
I eventually found the bdLoad documentation so that should help me understand the design and locate which transform and gate this happens at

Version 0, edited 10 years ago by lee (next)

comment:6 Changed 10 years ago by lee

Some more info on what I've found

Where this goes wrong is in writeAmd.getLayerText, the caller is writeDojo where it loops around each boot layer: writeDojo:

item.layerText= resource.layerText + writeAmd.getLayerText(item, item.layer.include, item.layer.exclude, true) + (item.bootText || "") + compat;

writeAmd.getLayerText:

return	(cache.length ? "require({cache:{" + newline + cache.join("," + newline) + "}});" + newline : "") +
	(resourceText===undefined ?	 insertAbsMid(resource.getText(), resource) : resourceText) +
	(resource.layer.postscript ? resource.layer.postscript : "");

The logic in writeAmd.getLayerText where 'resourceText===undefined' is false means the resourceText (which is an array of the layer includes), is appended to the layer.

Last edited 10 years ago by lee (previous) (diff)

comment:7 Changed 10 years ago by lee

Thanks to snover, this looks like it's related to #15856 which neonstalwart raised. It ties up with the fact I'm seeing writeAmd.getLayerText() called with 4 arguments but it's only taking 2.

comment:8 Changed 10 years ago by lee


My ticket has become a bit messy so in summary:

The issue only happens when building a boot layer which isn't the dojo layer.

if(item!==resource){

it only effects the uncompressed file output.

i.e. at http://bugs.dojotoolkit.org/browser/dojo/util/trunk/build/transforms/writeDojo.js#L152


the return from writeAmd.getLayerText

i.e. at http://bugs.dojotoolkit.org/browser/dojo/util/trunk/build/transforms/writeAmd.js#L218
appends an array to the layer return (through the resourceText arg)

Obviously the code has gotten out of sync with recent changes. I've attached the patch I made to get the layer to build properly

Changed 10 years ago by lee

Attachment: 15814.patch added

temporary "fix"

comment:9 Changed 10 years ago by Rawld Gill

Resolution: fixed
Status: newclosed

apologies for the delay; this should be a dup of #15856, which is now fixed.

comment:10 Changed 10 years ago by ben hockey

Milestone: tbd1.8.1
Note: See TracTickets for help on using tickets.