Gulpify shared/js#2402
Conversation
There was a problem hiding this comment.
Apps and shared are very similar. Might be some opportunity to share code, especially since eventually I'd like to get some of the JS in dashboard to also go through browserify, which means it will also likely need a step like this.
There was a problem hiding this comment.
Whitespace only changes to this file.
|
shared/js probably also deserves a README now. I will add that. |
There was a problem hiding this comment.
could these be provided by require statements now that this is bundled through browserify, or is it still being used by non-commonjs code? e.g.:
var $ = require('jquery');
var angular = require('angular');
There was a problem hiding this comment.
Quite possibly. I haven't actually touched the angular stuff yet. I think the downside to this is that dashboard probably already includes jquery, and this would result in a second copy of it in our bundle.
There was a problem hiding this comment.
If you want to dive into this, browserify-shim provides features for handling this type of transitional situation. See Expose global variables via global:*. For bundling code within Dashboard, we could use a config to shim the modules it expects to be provided on the global like this:
"browserify-shim": {
"jquery": "global:$",
"angular": "global:angular"
}
then you use require('jquery') and require('angular') within the modules. we could then use the same JS code outside of dashboard's environment (e.g. in js unit tests, or other bundles), loading the packages from npm dependencies.
commit 80025ca Merge: 347233c e39fa24 Author: Bjvanminnen <Bjvanminnen@gmail.com> Date: Thu May 14 09:25:35 2015 -0700 Merge pull request #2402 from code-dot-org/sharedGulp Gulpify shared/js commit 347233c Author: Continuous Integration <dev@code.org> Date: Thu May 14 15:56:42 2015 +0000 Automatically built. commit bacf9bc Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Thu May 14 08:49:02 2015 -0700 jshint fix commit bacf9bc Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Thu May 14 08:49:02 2015 -0700 jshint fix commit 2059e9e Merge: 2ea55ea a373318 Author: Bjvanminnen <Bjvanminnen@gmail.com> Date: Thu May 14 08:39:11 2015 -0700 Merge pull request #2414 from code-dot-org/linklocal2 use localpaths + linklocal to manage dependencies between packages in same repo commit a373318 Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Wed May 13 16:38:07 2015 -0700 use localpaths + linklocal to manage dependencies between packages in same repo commit 2ea55ea Author: Brent Van Minnen <bjvanminnen@gmail.com> Date: Thu May 14 08:31:16 2015 -0700 remove old references to canvas from readmes commit 804075a Merge: 313c224 4879f3c Author: Chris Pirich <chris@code.org> Date: Thu May 14 07:28:57 2015 -0700 Merge pull request #2398 from code-dot-org/applab_event_changes implement applab events with limited properties commit 313c224 Author: Will Jordan <will@code.org> Date: Wed May 13 18:54:52 2015 -0700 add international auto-redirects to varnish config commit 23f3b55 Author: Will Jordan <will@code.org> Date: Wed May 13 17:49:14 2015 -0700 add "^/poste" to Varnish cookie whitelist commit 26ed67b Author: Will Jordan <will@code.org> Date: Wed May 13 16:25:14 2015 -0700 fix typo commit 5c266f5 Author: Will Jordan <will@code.org> Date: Wed May 13 16:23:00 2015 -0700 Map /dashboardapi to studio.code.org/api instead of learn.code.org/api commit 6ee5e6b Author: Laurel Fan <laurel@code.org> Date: Wed May 13 17:05:18 2015 -0700 fix progress dots in header commit 61e1301 Merge: 34e7b8f bc27e2f Author: Andre Stackhouse <CaptainStack@outlook.com> Date: Wed May 13 16:53:16 2015 -0700 Merge pull request #2411 from code-dot-org/poste-dashboard-template Adding authentication back to this page for testing purposes. commit bc27e2f Author: CaptainStack <CaptainStack@outlook.com> Date: Wed May 13 16:52:21 2015 -0700 Adding authentication back to this page for testing purposes.
This does a couple of things
(1) Run initApp.js and related code through browserify using gulp as our task runner. We also get things like linting, and in the future can add tests.
(2) Add rake tasks that allow us to also build from root
(3) Mirror apps approach to how we generate our package (this will require you to add use_my_shared_js to your locals.yml if you want to build it). I don't love everything about this approach but it's at least semi-familiar.
#3 is the scariest in that it's the one I had the hardest time testing locally. I'll have to monitor the build process when this does get checked in as there might still be some bugs.
This doesn't yet set up any sharing between packages. I think my next step will be to consolidate clientApi usages across our packages.
I'm also going to add a bunch of line items to this PR.