angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.76k stars 11.98k forks source link

rollup #2817

Closed elvisbegovic closed 7 years ago

elvisbegovic commented 7 years ago

ENHANCEMENT:

Adding rollup ?!!

eight-molecules commented 7 years ago

We literally just went through this with SystemJS -> Webpack, and Webpack is going to work better for the generalized case that the CLI is designed to cover. This would be a major setback in getting to the Final release as large in-progress features such as AoT get paused, stripped out, and reimplemented to use Rollup.

Everyone has a favorite [insert tool here], but in this case you don't have to use the CLI if you don't like the workflow. Angular 2 isn't tied to it. I didn't use it when SystemJS was in, as I was using and preferred Webpack already. Had the project stuck to SystemJS, I still wouldn't be using it.

elvisbegovic commented 7 years ago

@gelliott181 dude i know that, there isn't better three shaking method than rollup so my issue is an enhancement or not ? by your comment I think now angular-cli do regression by passing to webpack without keep same funcs rollup !!! is this agile method 😁? Please apologize but stop tell me "dont have to use cli if don't like" thanks for response anyway

eight-molecules commented 7 years ago

This isn't about tree shaking being better in Rollup than in Webpack, or any specific feature. This isn't Agile (A development process, not a build tool). I'm not saying "Don't use the CLI." I also will not be apologizing for laying out an informed and reasoned disagreement with your feature, specifically pointing out just how far the Angular-CLI team would backtrack (Again) in their progress, along with Rollup not providing features Webpack has brought to the table.

The cons outweigh the pros. If we need the Rollup tree-shaking I have a feeling the CLI team is more than capable of integrating that with Webpack. Not only that, but we have Webpack plugins already available (TS: https://github.com/blacksonic/typescript-webpack-tree-shaking, Babel: https://github.com/blacksonic/babel-webpack-tree-shaking). While they're not perfect, that's the beauty of open source: If you find it deficient, you can help to improve it yourself.

With that, my final thoughts on this specific line of thought: We all have things we wish would happen, but every time the needs of the many will outweigh the needs of the few in these sorts of generalized tooling projects. When your needs diverge you can either continue to work with a large group of people all fighting for their needs to be heard, or you can take a fork in the road and find a way that works for you. Nothing in my statements has said you can't integrate Rollup into the CLI and submit a PR for the CLI maintainers to approve. I simply said the development team have made their decision and is now at a point where they're unlikely to back out as it takes an already delayed schedule and delays it further for little to no benefit for most users.

clydin commented 7 years ago

With webpack being blackboxed, it could potentially be swapped out for rollup or something else entirely in the future. Webpack just happens to be the internal build engine that the developers decided upon at this point in time. So using the CLI or not using the CLI solely based on webpack usage is probably not the best of ideas. And realistically (or maybe ideally), you shouldn't know the difference if it was swapped out in the future.

As to tree-shaking, rollup does seem to do a better job. However, the uglifyjs method employed by webpack (which is currently used for CLI production builds) seems to perform well enough. Combine that with the added infrastructure provided by webpack and the scale tips in its favor. (Atleast for now.)

eight-molecules commented 7 years ago

@clydin I'm glad you said that, as I am a huge proponent of the CLI given the massive set of features already available. I also agree that deciding for or against the CLI based on use of a specific tool is a bad way to make that decision, however it may be a major factor for some teams and devs who are required to use a certain workflow.

In this case though, it was an out of the blue request that a major component of the CLI's workflow be replaced, and the lack of a well reasoned argument for Rollup beyond "tree-shaking is better" makes that suggestion an arbitrary enhancement, unlike the current system which was done for specific reasons.

elvisbegovic commented 7 years ago

Incredible, 6 chars get you arrogant. I won't write a novel, don't want dedicate time for this. But, I'm not convicted (I know you will say you don't try to get me convicted) anyway, always think "adding rollup is must have feature". They are Google Developers, big contrubutors , webpack and ember cli tools... IMHO I expect more than that from them. BYE (yes I know I can stop use cli lol)

JohannesRudolph commented 7 years ago

I agree that unsubstantiated suggestions to replace a major component of the cli should be disregarded. However, I'm wondering about the claims that the current webpack implementation in the cli does not perform tree shaking? From a cursory glance at the aot bundles generated in - - prod mode this appears correct unfortunately...

webmutation commented 7 years ago

If you run ng build --prod or ng build --prod --aot it outputs removed functions so it is doing tree shaking, if you want to compare UglifyJs with Rollup tree shaking that is another thing (Rollup might perform better).

As far as i can tell CLI beta18 is performing tree shaking.

WARNING in main.641d46f2cf1bf4ab5876.bundle.js from UglifyJs Dropping unused function scheduleMicroTask [c:/DEV/cli-tools-exp/cli-test/~/@angular/compiler/src/facade/lang.js:21,0] Dropping unused function getTypeNameForDebugging [c:/DEV/cli-tools-exp/cli-test/~/@angular/compiler/src/facade/lang.js:28,0] Dropping unused function isDate [c:/DEV/cli-tools-exp/cli-test/~/@angular/compiler/src/facade/lang.js:47,0] Dropping unused function noop [c:/DEV/cli-tools-exp/cli-test/~/@angular/compiler/src/facade/lang.js:50,0] Dropping unused function looseIdentical [c:/DEV/cli-tools-exp/cli-test/~/@angular/compiler/src/facade/lang.js:101,0]

elvisbegovic commented 7 years ago

@webmaxru dude, thanks but I know that. lol (anyway right persons know about what i'm speakin)

mohammedzamakhan commented 7 years ago

I don't think this is an unreasonable request, even rollup is used for benchmarking angular 2 performance with other frameworks at https://github.com/krausest/js-framework-benchmark by @robwormald. Probably Rob can put more light onto whether using webpack for treeshaking or rollout with closure compiler?

filipesilva commented 7 years ago

We're not looking at changing the build engine again. For the foreseeable future we'll focus of making it as good as possible using webpack.

elvisbegovic commented 7 years ago

Why closing this enhancement! Im not telling change build engine but only tree shaking method ! So maybe help webpack team to get webpack using or do better than rollup... I was thinking they are on it ! Still not understand why close enhancement request

robwormald commented 7 years ago

@istiti what you're asking for would be a feature request to webpack, and such functionality is already being discussed here https://github.com/webpack/webpack/issues/2873

elvisbegovic commented 7 years ago

that I agree, maybe better write request to webpack

michael-jennings commented 7 years ago

fans self with magazine jer-RY! jer-RY! jer-RY!

elvisbegovic commented 7 years ago

?

angular-automatic-lock-bot[bot] commented 5 years ago

This issue has been automatically locked due to inactivity. Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.