kasper / phoenix

A lightweight macOS window and app manager scriptable with JavaScript
https://kasper.github.io/phoenix/
Other
4.36k stars 128 forks source link

- Build fails if you don't have uglify-js installed. #232

Closed cfraizer closed 5 years ago

cfraizer commented 5 years ago

I updated the README.md, but you might want to re-word or move that to a different spot.

jasonm23 commented 5 years ago

@kasper Perhaps we can just remove Uglify as a dependency?

kasper commented 5 years ago

Whoops, no the correct approach would be to add npm install to the build phase if NPM is discovered (which it already checks).

jasonm23 commented 5 years ago

@kasper what is Uglify doing in the build, is it essential?

kasper commented 5 years ago

@jasonm23 Yes, it’s part of building the JavaScript library, check package.json. There are other dependencies there as well such as linting. It is only built if you have Node installed locally.

jasonm23 commented 5 years ago

Sorry, I should clarify where I'm coming from.

I can see package.json is using Uglify as part of build, but what I'm having an issue with understanding is why would we need to minify/Uglify the JS.

It seems like a redundant step, since minification only has benefit when the code is being served online. (Due to lower bandwidth).

Since that's not a concern with Phoenix, do we really need Uglify?

kasper commented 5 years ago

It makes the bundle size smaller. Having the dependency is not really the issue here. If you are missing for example XO, you will not be able to lint either. So the installation step is missing from the build phase.

jasonm23 commented 5 years ago

Fair. Automated installation of the dependencies is the real solution, as you say @kasper.

kasper commented 5 years ago

Fixed in 3893d1b9a28d6e7315909de1e3147a93a9b8a2c3. Thanks @cfraizer for noticing this!