Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script should run synchronously #499

Closed
samreid opened this issue Sep 26, 2016 · 7 comments
Closed

Script should run synchronously #499

samreid opened this issue Sep 26, 2016 · 7 comments
Assignees

Comments

@samreid
Copy link
Member

samreid commented Sep 26, 2016

From phetsims/scenery#567

I tried adding a build-js task to build scenery and include it in a sample HTML file (test.html in scenery's chipper branch). However, when the test.html starts up, window.scenery is undefined until after one setTimeout. This is caused by the asychronous e(['']) being used instead of the synchronous e(''). requirejs/almond#42 reports how we can use wrap to guarantee a synchronous load.

I tested the wrap strategy in phetsims/scenery#567 and it appeared to work properly for scenery test.html as well as for a FAMB build. I'll commit shortly then assign for review.

@samreid samreid self-assigned this Sep 26, 2016
samreid added a commit that referenced this issue Sep 26, 2016
@samreid
Copy link
Member Author

samreid commented Sep 26, 2016

I committed the change above, @jonathanolson or @andrewadare or @pixelzoom do you have time to review?

@pixelzoom
Copy link
Contributor

Sorry, I don't have time to look at this one.

@pixelzoom pixelzoom removed their assignment Sep 26, 2016
@andrewadare
Copy link
Contributor

@samreid There's not much to review here; if you tested it and it works, I'm not sure how exactly I can be of assistance. The new source looks fine to me, although I'm not sure whether the generated code should be following the PhET formatting guidelines (space padding in parens).

@andrewadare andrewadare removed their assignment Sep 26, 2016
@samreid
Copy link
Member Author

samreid commented Sep 26, 2016

I'm not sure how exactly I can be of assistance

Perhaps do a build with and without it on a different project (using --uglify=false) and make sure they differ as prescribed, and double check that something builds and runs nicely?

I'm not sure whether the generated code should be following the PhET formatting guidelines

I do not know whether wrap or uglify goes first. In either case, the code is intended for build, so would be embedded in HTML.

@samreid samreid assigned andrewadare and unassigned jonathanolson Sep 26, 2016
@andrewadare
Copy link
Contributor

andrewadare commented Sep 27, 2016

Here's a build of CLB diffed between e6236b6 and today's master:

[capacitor-lab-basics]$ diff clb-chipper-e6236b6.html clb-chipper-master.html
--- clb-chipper-e6236b6.html    2016-09-27 08:50:44.000000000 -0600
+++ clb-chipper-master.html 2016-09-27 08:47:34.000000000 -0600
@@ -38,7 +38,7 @@
   window.phet.chipper = {};
   window.phet.chipper.project = 'capacitor-lab-basics';
   window.phet.chipper.version = '1.0.0-dev.28';
-  window.phet.chipper.buildTimestamp = '2016-09-27 14:50:44 UTC';
+  window.phet.chipper.buildTimestamp = '2016-09-27 14:47:34 UTC';
   window.phet.chipper.brand = 'phet';
   window.phet.chipper.locale = 'en';

@@ -540,7 +540,7 @@
 };
   // ### END THIRD PARTY LICENSE ENTRIES ###
   window.phet.chipper.dependencies = {
-  "comment": "# capacitor-lab-basics 1.0.0-dev.28 Tue Sep 27 2016 08:50:43 GMT-0600 (MDT)",
+  "comment": "# capacitor-lab-basics 1.0.0-dev.28 Tue Sep 27 2016 08:47:33 GMT-0600 (MDT)",
   "assert": {
     "sha": "845be6b591cb746b0b05b7023208aef2e49a8855",
     "branch": "master"
@@ -562,8 +562,8 @@
     "branch": "master"
   },
   "chipper": {
-    "sha": "e6236b6a2ca33377168c40318f73756054b5ae62",
-    "branch": "HEAD"
+    "sha": "589bb2f9e98731e04fd369da279befb6b5bff20c",
+    "branch": "master"
   },
   "dot": {
     "sha": "879674e2590f057a1f099322875561d31503d855",
@@ -18617,7 +18617,7 @@
     };
   })();
 </script>
-<script type="text/javascript">(function () {/**
+<script type="text/javascript">(function() {/**
  * @license almond 0.2.9 Copyright (c) 2011-2014, The Dojo Foundation All Rights Reserved.
  * Available via the MIT or new BSD license.
  * see: http://github.com/jrburke/almond for details
@@ -91529,8 +91529,7 @@
 } );


-
-require(["capacitor-lab-basics-main"]);
+require('capacitor-lab-basics-main');
 }());</script>
 </body>
 </html>

So it looks like the square brackets made it in were removed correctly.

Both versions run locally without any problems.

@andrewadare andrewadare assigned samreid and unassigned andrewadare Sep 27, 2016
@samreid
Copy link
Member Author

samreid commented Sep 27, 2016

Thanks @andrewadare, closing.

@samreid
Copy link
Member Author

samreid commented Dec 14, 2016

On Oct 21, @phet-steele identified a problem that using the synchronous main caused failures for the Chrome "Save As..." for saving a simulation, which would make it so the sim wouldn't launch properly. After reverting this change to make main asychronous again, we are again able to save html sims and launch them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants