Opened 7 years ago

Closed 7 years ago

Last modified 7 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 7 years ago.
build profile
blah.profile.js (138 bytes) - added by lee 7 years ago.
package profile
package.json (130 bytes) - added by lee 7 years ago.
blah package json
15814.patch (877 bytes) - added by lee 7 years ago.
temporary "fix"

Download all attachments as: .zip

Change History (14)

Changed 7 years ago by lee

Attachment: build.profile.js added

build profile

Changed 7 years ago by lee

Attachment: blah.profile.js added

package profile

Changed 7 years ago by lee

Attachment: package.json added

blah package json

comment:1 Changed 7 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 7 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 7 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 7 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 7 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 bdBuild documentation so that should help me understand the design and locate which transform and gate this happens at

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

comment:6 Changed 7 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 7 years ago by lee (previous) (diff)

comment:7 Changed 7 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 7 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 7 years ago by lee

Attachment: 15814.patch added

temporary "fix"

comment:9 Changed 7 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 7 years ago by ben hockey

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