Skip to content

Conversation

@YellowSharkMT
Copy link

Just an idea for implementing the ng-annotate package as part of the JS minification, so that Angular scripts can be minified without trouble. (More info on the issue: https://scotch.io/tutorials/declaring-angularjs-modules-for-minification).

Not sure that I've got the optimal implementation here yet; I was kicking around the idea of making a separate call for it as part of the webmakefile, but Angular annotation really isn't required outside of the scope of minification, so it just seemed easier to drop it into the actual minify_js() function. Another idea I had is to make a setting/argument that toggles the Angular annotation, as opposed to running it on all scripts that get minified.

Let me know what you think, if you'd be interested in this functionality, any tidy-ups, etc. Thanks!

@bazzisoft
Copy link
Owner

Thanks for the pull request @YellowSharkMT . I haven't used angular for a few years (angular 2 not at all) so this is all a bit new to me. I don't think modifying minify_js() is the right approach, as that will affect non-angular source files as well. Perhaps you could try creating a separate module modules/angularjs.py specifically for minifying angular 2? It could pass the input files through concatenate, then ng-annotate, then uglify-js (or use a simpler process if one exists). That could be exposed as a separate minify_angular2() function in the API. What do you think?

-- Barak

…u 18.10

- Bugfix: the `--no-bin-links` flag for `npm install` causes the `.bin` directory to *not* be created.
Conflicts:
	webmake/modules/copyfiles.py
	webmake/modules/minify.py
	webmake/modules/reactjs.py
	webmake/package.json
- Adding `annotate_angular` kwargs to `api.minify_js`
- Adding same kwarg to `moduls.minify_js`, using it to conditionally implement the annotations
@YellowSharkMT
Copy link
Author

@bazzisoft just updated my fork from your branch - basically my package is an exact copy of yours, except for the following:

  • api.minify_js: adding annotate_angular param
  • modules.minify.minify_js: adding annotate_angular param, runs ng-annotate if true
  • packages.json: I had to bump node-sass to version 4, I encountered problems getting node-sass 3 to install on my Ubuntu systems (16.04 & 18.10). Probably a missing lib of some sort. Odd though, since version 4 doesn't seem to have same problems.

I'd forgotten that this pull request was open, actually - you can feel free to ignore it, even close it if you want, etc. No big deal, I'm cool either way. Alright thanks!

@bazzisoft
Copy link
Owner

bazzisoft commented Feb 19, 2019

Hey @YellowSharkMT , glad you are getting some value out of this project! I appreciate the pull request and yet I'm hesitant to merge our stuff together because it sounds like we have somewhat different asset pipelines, what with the angular vs react stuff.

This is a tricky project as I'm trying to make it as least configurable as possible rather than generic, to match a certain workflow at a point in time. The down side is that every time I introduce any potentially breaking change (even version bumping the sass lib for example) there needs to be a new major version to ensure it won't break any historic projects depending on the older tools/workflows.

So I think it may be best to leave your version as a separate fork with the angular support as you need it, and we can sync up with each other (even just parts of the code) when we push a new major release.

Btw some of your changes (specifically the node dir fixes) look like good candidates for inclusion in my next version bump. I had it like that originally to get around some node/npm issues but maybe they are resolved now with newer versions of those, I will test.

What do you think?

-- Barak

YellowSharkMT and others added 14 commits March 19, 2019 23:21
…on objects.

- Adminx: job details page, bulk import handler, view tests
- Buyer order form (templates & js)
- Buyer labels editor popup (templates & js)
- Buyer job details
- Various buyer form/view updates
- Common: label update handler
…grade-v2.2.0

Conflicts:
	webmake/package.json
…lves an issue that came up on a project that uses webmake, although I'm not certain of the exact bit of LESS that's causing the issue.
Mangle doesn't seem to be compatible out-of-the-box with annotate angular
Remove the mangle arg from uglifyjs call
Hotfix: Remove the mangle arg from uglifyjs call
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

Successfully merging this pull request may close these issues.

2 participants