-
-
Notifications
You must be signed in to change notification settings - Fork 643
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
Bundle dependencies and code using Webpack #869
Comments
Definitely. I have something half-built in a branch but I haven't had a chance to get back to it. |
Couple issues remain.
Once these tasks are done, bower can be removed from DIM. I've added the appropriate commands to build the generated file to npm on install. We're at the point now where we can consider bringing in the scripts into webpack and chunking the export with vendor/app files, setting up a watcher to recompile the app files when they change, and building with ES6/Babel. I'm not sure we should refactor what we have to ES6... |
👋 Happy to help out where I can! Webpack's the sort of thing I guess where its very easy to step on other people's toes and waste effort, so I'm happy to do as much (or little) to move this forward, given some direction. |
Nice! This is a great step. I agree that we should get babel going - we can use the plugin to auto select the minimum transforms to support our target platforms, plus babili for modification and ng-annotate for automatic $inject. I can work on that if you're not already doing it - I got stalled because I didn't have webpack working yet.
…-Ben
On Dec 29, 2016, at 4:32 AM, Josh Hunt ***@***.***> wrote:
👋 Happy to help out where I can! Webpack's the sort of thing I guess where its very easy to step on other people's toes and waste effort, so I'm happy to do as much (or little) to move this forward, given some direction.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
It's also worth testing if combining scripts actually helps performance. With HTTP/2, it's generally suggested to not concatenate since multiple small requests can be handled better that a large one.
…-Ben
On Dec 29, 2016, at 8:43 AM, Ben Hollis ***@***.***> wrote:
Nice! This is a great step. I agree that we should get babel going - we can use the plugin to auto select the minimum transforms to support our target platforms, plus babili for modification and ng-annotate for automatic $inject. I can work on that if you're not already doing it - I got stalled because I didn't have webpack working yet.
-Ben
> On Dec 29, 2016, at 4:32 AM, Josh Hunt ***@***.***> wrote:
>
> 👋 Happy to help out where I can! Webpack's the sort of thing I guess where its very easy to step on other people's toes and waste effort, so I'm happy to do as much (or little) to move this forward, given some direction.
>
> —
> You are receiving this because you were assigned.
> Reply to this email directly, view it on GitHub, or mute the thread.
>
|
@joshhunt What should we be looking out for? We're incrementally updating our build process so atm all of our code is ES5. |
First up, Webpack 2 is in a pretty stable now (I think its in RC or something, I've been using a much earlier beta in a project for a while and its fine), so I would recommend we go with that from the start. There's not too much different (in terms of config and using it) from Webpack 1, so it should be fine. Docs are on this new site. Personally, I think the priority is to bundle everything together first, building your dependency graph using imports/requires. This would require 'minimal' code changes to the application, but we would have to go through and make sure that every source file and external dependency is 'required' from some place. There's a bunch of ways to architect this for an Angular app (or so I've read, I don't really use Angular all that much), but I think the best way for now would be to treat... import "scripts/dimApp.config";
import "scripts/dimApp.i18n";
import "scripts/services/dimActionQueue.factory";
import "scripts/services/dimBungieService.factory";
import "scripts/services/dimDefinitions.factory";
import "scripts/services/dimManifestService.factory";
angular.module('dimApp', [
'ui.router',
'timer',
...
]) We would also need to import at least Angular at the top of every file (that uses it). Technically you could use Adding a babel config to pop in ES6+ syntax as we go would also be a great idea. And then I guess a switch to generate 'production' and 'development' builds (to minify) would also be recommended. This might sound like a lot, but I think it's fairly straightforward. That should be the target for the 'first release' of webpack. After that we can work on reaping the benefits like seperate vendor builds, ng-annotate, directive templates, etc. I'm happy to build on what's been done here and take a stab at the above, as long as I don't step on anyone toes. I should be able to get something solid done today, once I finish the raid 😉 |
Oh, few other random points:
|
@joshhunt Go for it. The branch is up to date so it's yours to build upon. |
FYI, the zip.js module has js files that are loaded by the zip.js file when the code needs the feature.. These can't be front-loaded in the bundle. I was going to try to copy the files during a build step to the vendors folder as we do now, but without the use of bower. |
I like everything you're saying here !
…-Ben
On Dec 29, 2016, at 6:17 PM, Rick Casey ***@***.***> wrote:
FYI, the zip.js module has js files that are loaded by the zip.js file. These can't be front-loaded in the bundle. I was going to try to copy the files during a build step to the vendors folder as we do now, but without the use of bower.
―
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Also, not sure if it's clear, but we're using es6 extensively in the existing code.
…-Ben
On Dec 29, 2016, at 6:17 PM, Rick Casey ***@***.***> wrote:
FYI, the zip.js module has js files that are loaded by the zip.js file. These can't be front-loaded in the bundle. I was going to try to copy the files during a build step to the vendors folder as we do now, but without the use of bower.
―
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Couple of issues that remain on the webpack branch.
The following html files do |
@joshhunt I tried to get this working but something about |
Yeah... you need to be careful when doing dynamic requires with Webpack. If
you make this a simple expression in the form of 'require('views/' +
viewname + '.template.html')' it *should* work. Gotta make sure the
templates have the extension .template.html so they get loaded correctly.
Otherwise if the dynamic require/context doesn't work, you could just
always great an object of all the languages that way webpack has static
requires that it can understand easier.
Or I'll check it out tomorrow in the new year :)
…On Sat, 31 Dec 2016 at 4:53 pm, delphiactual ***@***.***> wrote:
@joshhunt <https://github.com/joshhunt> I tried to get this working but
something about context...
https://github.com/DestinyItemManager/DIM/blob/dev/app/scripts/shell/dimAppCtrl.controller.js#L141
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#869 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC0PscD9W6VN_Nuop3oDZuU-aKgBF5Fks5rNe26gaJpZM4J4UU1>
.
|
I figured it out, I was missing the |
Closing this because we're well on our way! |
A LONG TERM goal for DIM was to start bundling our dependencies. This has come consequence that needs to be considered.
The text was updated successfully, but these errors were encountered: