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

Clean dev dependencies #985

Closed
mgechev opened this issue Jun 12, 2016 · 22 comments
Closed

Clean dev dependencies #985

mgechev opened this issue Jun 12, 2016 · 22 comments
Labels

Comments

@mgechev
Copy link
Owner

mgechev commented Jun 12, 2016

From our initial goal to keep the seed minimalistic, it's set of dependencies grew over the last a couple of months. This made the entire installation process much longer, bringing a couple of extra plugins which may or may not be used by many.

I'm opening this issue with suggestion to drop:

  • audoprefixer
  • colorguard
  • doiuse
  • gulp-clean-css
  • gulp-postcss
  • gulp-progeny
  • gulp-sass
  • gulp-sass-lint
  • karma-phantomjs-launcher (we already run tests in chrome)
  • postcss-reporter
    -stylelint
  • stylelint-config-standard

I'd love to hear more thoughts. Do you really use all/any of these plugins or you prefer to keep the installation process lighter and get rid of them (or some of them)?

Note that there's a version of the seed which includes more functionality and respectively more dependencies here.

// cc @ludohenin @d3viant0ne @TheDonDope @NathanWalker @Shyam-Chen

@ckapilla
Copy link
Contributor

The leaner the better! I personally would be happy to kiss all those good bye.

@mgechev
Copy link
Owner Author

mgechev commented Jun 12, 2016

For reference:

Original seed:

$ time npm install
...
real    3m59.031s
user    1m33.492s
sys     0m22.507s

Minimalistic seed:

$ time npm install
...
real    2m46.400s
user    1m14.654s
sys     0m18.166s

@corvinusy
Copy link

Availability of 2 variants would be fine.

@mgechev
Copy link
Owner Author

mgechev commented Jun 12, 2016

Supporting three versions of the seed (minimalistic, the current one and the advanced seed of @NathanWalker) seems like a lot of effort, that I'm not sure is worth it. The minimalistic seed could be turned into the current version by adding Sass and css linting, which if required, is matter of a couple of hours.

@Shyam-Chen
Copy link
Contributor

I would like to use rucksack-css instead of autoprefixer.

@TheDonDope
Copy link
Contributor

Hi!
I am a little on the fence about this topic. While i do support the strive for the least amount of dependencies, i think we shouldn't just try to throw out dependencies for lesser install time sake.

For example:

  • gulp-progeny
  • gulp-sass
  • gulp-sass-lint

Those dependencies got included when @hank-ehly inlcuded the sass build option. People were happy to be able to use sass by a simply enabling a configuration flag. If we were to drop those, we would in consequence need to revert this build option too.

Surely we cannot make everyone happy and there will always be some "I want to use xyz instead of abc" or "Can't we integrate foo into the seed?". I think we need to be more precise and strict about what the aim of the seed is. Basically i see two options here:

A) Absolute bare minimum seed

  • Keep all dependencies to the absolute bare minimum
  • Provide more wiki articles on integration of further 3rd party libraries

Pro

  • Less dependencies = less maintenance

Con

  • Wiki articles should be updated more frequently

B) Provide a seed for the 80% use-cases

  • Provide a configuration of dependencies/ tasks/ tools that will fulfill the needs of "the majority" (taking sass as an example again: this was heavily requested)
  • Less work for the people who use the seed to get an environment running which serves them well

Pro

  • Help people getting started quicker

Con

  • The needs of "the majority" is a wiggly term, getting a consensus on what should be included might be difficult

That are my thoughts on that.

Greetings

Dope

@sebald
Copy link

sebald commented Jun 13, 2016

I would much prefer a minimalistic seed that only handles bundling and testing the script part.

For everything else: Maybe a collection of receipts of "How do I add x?" would be nice and help everyone to setup projects to their needs?

@mgechev
Copy link
Owner Author

mgechev commented Jun 13, 2016

@sebald I'd appreciate your feedback for the minimalistic branch.

@sebald
Copy link

sebald commented Jun 13, 2016

@mgechev It certainly looks like a loot of stuff, but almost everything is needed. Exceptions are (imho):

  • autoprefixer: Because if you use Sass/Less/Stylus you won't need it.
  • cssnano: Never used it, not quite sure if needed if you're not using other CSS-precompiler.

The hard part, if think, is that it looks so damn much stuff. But in reality it isn't. You removed all the "unnecessary" stuff in 467e6f2#diff-b9cfc7f2cdf78a7f4b91a753d10865a2 already.

I really like that you hide tasks than actually run gulp with npm scripts. It good to have only on "task runner". Maybe going all the way would be an option? Meaning, hiding that gulp is used at all and instead of having a task to list all gulp tasks use this https://github.com/srph/npm-scripts-info ? I also used https://www.npmjs.com/package/npm-run-all in the past, which is super helpful :)

@TheDonDope already mentioned it. Updating the wiki more frequently. It may be hard to keep it updated with the pace this seed changes, but it is also super important, because let's be honest: we just wanna leech clone your work and have it working flawlessly! ;)

...and because I do not wanna end my post with a negative statement: Dude, I have used this seed back in den Angular Beta so so many times to see how the API changed and play around with the upcoming Angular. Even if people do not use or appreciate this seed, b/c there is ng-cli, it is such a fantastic learning resource! Finally I know how to get coverage reports with TS! Thanks a lot for all the work and heart you put into this.

@mgechev
Copy link
Owner Author

mgechev commented Jun 13, 2016

Thanks for the feedback (it wasn't negative at all, it was quite constructive :-))! Currently we use postcss because of autoprefixer (it's quite useful to set a list of supported browsers and get prefix for your styles automatically) and cssnano.

We're not competing with angular-cli. The CLI team is doing great things! I'm contributing to the project with ideas around the style guide + tools for static code analysis. On the other hand, there are a lot of people who prefer minimalistic starter which is less opinionated compared to the CLI - this seed is a good start for developers with such preferences.

@ludohenin
Copy link
Collaborator

I'm also on the side to keep it minimalistic.
We'll, of course, have to provide a bunch of recipes / hooks or even ready to use tasks for the common use cases.
Things like Sass integration is great and was a really requested feature, on the other hand I feel that it messes up the code a bit.

@hankehly
Copy link
Contributor

@mgechev

If the goal is to be minimalistic and only require what is basically necessary, sass is not necessary and could be removed; although, I am sad to see it removed only 2 weeks after creating it.

I would like to request that the goal of the repository be made more apparent. The word minimalistic doesn't appear in the README. I think being more explicit about the goal of the repository will help future contributors understand what modifications are and aren't desirable.

@mgechev
Copy link
Owner Author

mgechev commented Jun 15, 2016

I'd suggest to apply the minimalistic approach and add/update the information for:

  • Sass integration.
  • AngularFire2 integration.
  • Mobile-toolkit integration.
  • Angular 2 JWT.
  • AngularMaterial 2.

@joshwiens
Copy link
Contributor

Why don't we finishing the current refactoring and get it stable before we start gutting other stuff out.

The number of major changes is approaching excessive for anyone that has to update forks/projects every time something gets rearranged and or pulled.

As to what is proposed to be removed, it's useful to any project with a stylesheet so it doesn't make a great deal of sense to me but w/e.

As to why it's being removed, you can attribute a great deal of that time increase to both npm the service, npm the package, your internet and the system you are installing on.

My time npm install is just under 3 minutes with current master with Node 6 running on OSX.

Point being, finish the major work that has lots of projects holding features & fixes until it lands before ripping out packages people probably use daily and will have to add back for the sake of ~75 seconds saved occasionally.

@mgechev
Copy link
Owner Author

mgechev commented Jun 15, 2016

Yes, it'll be best to have #959 in master before stripping dependencies.
The maintenance cost is bigger problem compared to the installation duration.

@spicemix
Copy link

The finest "recipe for adding functionality X" is a working, tested, maintained version with functionality X. So I don't see how forcing people to constantly update docs is easier or better for anyone than going ahead and just updating code. This is why comments/jsdoc have fallen away in usage and been replaced by test suites for teaching people how things work and why. Text goes stale too easily.

It's also hard for me to imagine people wanting a truly "minimal" seed (wouldn't the CLI be that?). Why would someone invest the time in learning an mind-numbingly complex web framework like NG2 but not want advanced CSS processing like Sass/PostCSS? "Lightweight" is a buzzword, people actually want everything to "just work."

Encouraging branches & forks rather than discouraging them seems the only reasonable option if this is too much maintenance or install time for you. Eventually maybe someone will make a configurator tool that lets you check off what components you want or don't in your starter. But the code must be present and running to enable that.

Thanks so much for your brilliant work!

@tarlepp
Copy link
Contributor

tarlepp commented Jun 19, 2016

So where to draw those lines?

Some people like bootstrap and others material design, and so one... Imho there is no "right" way to handle these in the seed.

@spicemix
Copy link

I think the issue here (if I can read Minko's mind) is he doesn't want to feel responsible for breaking someone's branch or fork. So he wants only two to worry about, one minimal (his) and one "advanced" (Nathan's).

Fine but the idea of open source is it lets you, in the words of one wag, "Move fast and break things." So everyone can decide what they want to worry about and let the crowd figure the other things out. I don't see his responsibility to add-on developers as particularly more burdensome than to end-users (the nature of this project discourages breaking anything arbitrarily). And I think developing an add-on to the seed would be a nifty little project for people, with its snacks of keeping up with updates.

So let a thousand flowers bloom and git and npm will let us roll back to the last known stable version of whatever component we want that hasn't been maintained promptly. Everyone can decide how much effort and responsibility they want and pick up where someone else got bored and slagged off. I'm fine with stripping any running code out of master, I just hope it goes somewhere better than /dev/null. And it would be nice for the mother to encourage that. =)

@joshwiens
Copy link
Contributor

joshwiens commented Jun 19, 2016

To @tarlepp 's point, the line is actually quite simple in my mind.

This is a set of tools to facilitate and simplify rolling out a new project in Angualr2.
The seed need only be capable of supporting things like bootstrap, material2, sass, less, blah blah blah.

Looking at what is proposed to be removed, examine the function of the lib.

Anyone know of an Angular2 project of any scale that doesn't use stylesheets?

  • audoprefixer <--- Anyone not have to support browsers outside of latest chrome?
  • colorguard <--- This one could be removed, it's fluff.
  • doiuse <--- This one could be removed, it's also fluff.
  • stylelint <--- This is akin to removing tslint, the argument that the IDE handles linting is moot as there are linting errors currently in seed master.
  • stylelint-config-standard <--- Small and goes with the above
  • gulp-clean-css <-- Executes all of our stylesheet optimizations, Anyone deploy to prod without optimized stylesheets?
  • gulp-postcss <-- Controls all of our stylesheet optimizations, Anyone deploy to prod without optimized stylesheets?
  • postcss-reporter <--- This is fluff but helpful so long as we are going to lint styles. It's also tiny.
  • gulp-progeny <--- Necessary so long as we support sass import without this are a nightmare

You could rip out these along with the "sass" flag and go back to just instructions

  • gulp-sass
  • gulp-sass-lint

Would it not make more sense to rip out chorme & test everything headless

  • karma-phantomjs-launcher

All that said, none of this makes any difference to me at this point and I have no real argument for or against other than the logical one above.

@mgechev
Copy link
Owner Author

mgechev commented Jun 20, 2016

@d3viant0ne the minimalistic branch contains the dependencies you've mentioned except the phantomjs-launcher. Running the tests in this headless browser requires a binary which needs to be downloaded both by us and CI. Also, currently we're running tests only in Chrome so phantomjs is not used.

@mgechev
Copy link
Owner Author

mgechev commented Jun 22, 2016

@d3viant0ne l did a small step which will not introduce breaking changes but on the other hand will speed-up the initial installation quite a bit! Since we're primary using Chrome, I suggest to drop the PhantomJS launcher and keep the Sass support for now #1027.

@mgechev
Copy link
Owner Author

mgechev commented Jul 4, 2016

After the discussion I believe that we concluded that dropping the Sass support will bring only more questions and confusion. So lets keep the seed like it is and close this issue :-).

@mgechev mgechev closed this as completed Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests