Opened 3 years ago

Closed 3 years ago

#18919 closed defect (fixed)

using latest closure compiler breaks build

Reported by: gerhard presser Owned by:
Priority: blocker Milestone: 1.12
Component: BuildSystem Version: 1.12.0-rc2
Keywords: Cc:
Blocked By: Blocking:

Description

using the latest build-tools packed with 1.12.rc2, when using closure-compiler, the dojo.js-result file breaks something in my app.

I get following error when loading just a simple page with a dijit/Button & dijit/ValidationTextBox

dojo.js.uncompressed.js:1287 Uncaught TypeError?: singleton.watch is not a function

at http://localhost:8001/wf/scripts/dijit/focus.js?en-us_9.0_1649267441663_Europe/Berlin:366:12 at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:473) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at ea (http://localhost:8001/wf/scripts/dojo/dojo.js:19:184) at http://localhost:8001/wf/scripts/dojo/dojo.js:20:220

It seems that mixing the Stateful in doesn't work!?

I didn't change any content of the dojo.js layer.

This doesn't happen when using closureCompiler.js from the 1.11 release.

Attachments (5)

build.profile.js (1.0 KB) - added by Michael J Van Sickle 3 years ago.
test build profile
index.html (649 bytes) - added by Michael J Van Sickle 3 years ago.
test index.html
ep.profile.js (1.4 KB) - added by gerhard presser 3 years ago.
layers.zip (138.1 KB) - added by gerhard presser 3 years ago.
build-report.txt (87.3 KB) - added by gerhard presser 3 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 3 years ago by dylan

Milestone: tbd1.12
Priority: undecidedblocker

Changed 3 years ago by Michael J Van Sickle

Attachment: build.profile.js added

test build profile

Changed 3 years ago by Michael J Van Sickle

Attachment: index.html added

test index.html

comment:2 Changed 3 years ago by Michael J Van Sickle

I've uploaded a build profile and index.html file (assumed to be stored in a folder called 18919 that is a sibling to the dojo root packages) and they build without issue. The only change I had to make was to use dojo's master branch since there was a regression in the dojo/strings module in tags/1.12.0.rc2.

@gpresser, can you upload a similar build profile / HTML page combination that demonstrates the issue so that I can address it?

Thanks!

Changed 3 years ago by gerhard presser

Attachment: ep.profile.js added

Changed 3 years ago by gerhard presser

Attachment: layers.zip added

comment:3 Changed 3 years ago by gerhard presser

I uploaded my build profile and the resulting layers.

the *-new.js layers are the result using the latest compiler.jar the *-old.js layers are the result using the dojo compiler.jar shipped with dojo 1.10.2

my testpage looks quite the same as yours. just a simple login-screen. no need to upload it

Changed 3 years ago by gerhard presser

Attachment: build-report.txt added

comment:4 Changed 3 years ago by Michael J Van Sickle

I've tracked this down to the forceNew function in dojo/_base/declare. It appears that the new closure compiler is eliminating the line that is assigning the prototype (xtor.prototype = ctor.prototype). Trying to find how to convince the compiler to not optimize that away.

comment:5 Changed 3 years ago by Michael J Van Sickle

I've been able to address the initial issue, but have run into another issue where it appears that the compiler is being overagressive in its optimizations. In this case it appears to be skipping the call to the _FormWidget#postMixInProperties method when instantiating a dijit/form/Button programmatically. It is starting to appear that the changes landed in https://github.com/dojo/util/pull/45 should be reverted until a more comprehensive analysis of the impact can be performed.

comment:6 Changed 3 years ago by Michael J Van Sickle

After exploring more, I discovered that the compiler also removed the call to postMixInProperties from dijit/_WidgetBase#create. Since this is a vital part of many widgets' life cycles, this is obviously unacceptable. Further, it raises concerns that more regressions might appear through a built application. Due to the resources required to fully vet the Dojo codebase and update it to be compatible, I am recommending that we revert https://github.com/dojo/util/pull/45 via https://github.com/dojo/util/pull/50 until the impact of the update can be assessed.

comment:7 Changed 3 years ago by gerhard presser

reading about all the new findings, this sounds reasonable to me. thx for your investigations.

comment:8 Changed 3 years ago by Michael J Van Sickle

@gpresser, I have worked on this some more. Can you test your application using the master branch of the util repo and see if this results in a successful build? Info about the change can be found here: https://github.com/dojo/util/pull/50

comment:9 Changed 3 years ago by Michael J Van Sickle

Resolution: fixed
Status: newclosed

comment:10 Changed 3 years ago by gerhard presser

I'll give it a try today...

comment:11 Changed 3 years ago by gerhard presser

unfortunately I can still reproduce the very same issue using the new compiler.jar & optimizeRunner.js

comment:12 Changed 3 years ago by dylan

Resolution: fixed
Status: closedreopened

@gpresser , we're finding that everything is working as expected in our recent tests. I'm wondering if it's possible that your tests did not include the recent updates, or if there's something different about your environment vs. ours.

I've gone ahead and built an RC3 with all the recent changes. We would definitely like to get this solved as it's the last blocker for 1.12!

comment:13 Changed 3 years ago by sindilevich

@dylan, @mvansickle, @gpresser I can still reproduce the same issue too. I think the deal is that the above commit has fixed it in the build/optimizeRunner.js file, without taking care of the build/transforms/optimizer/closure.js file. According to this comment (https://github.com/dojo/util/pull/27#issuecomment-127580856): "Changes made in this area will also have to be applied twice. Once to optimizeRunner.js (used in a 'Node build' when spawning a JVM to run closure): build/optimizeRunner.js. And once to closure.js (used in a 'Java build'): build/transforms/optimizer/closure.js".

I run a Java build for Dojo, using the dojo/util/buildscripts/build.bat with Java 7. Such builds do not have any of the recent changes, made for Node builds.

I could re-check the issue, when you bring the build/transforms/optimizer/closure.js on par with the Node-version.

Last edited 3 years ago by sindilevich (previous) (diff)

comment:14 Changed 3 years ago by rgillan

Build worked for us using the latest RC3

comment:15 Changed 3 years ago by dylan

Current plan is to land the Java version fix on Wednesday and release RC4 on Wednesday. For now it looks like the Node.js version works fine.

comment:16 Changed 3 years ago by dylans <dylan@…>

In 21c3bf8f/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:17 Changed 3 years ago by dylans <dylan@…>

In b76a01c/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:18 Changed 3 years ago by dylans <dylan@…>

In 7a42a3d/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:19 Changed 3 years ago by dylans <dylan@…>

In e27ef3b4/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:20 Changed 3 years ago by dylans <dylan@…>

In 2e73c71/util:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:21 Changed 3 years ago by dylan

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.