search

angulardart / angular_components

The official Material Design components for AngularDart. Used at Google in production apps.
https://pub.dev/packages/angular_components
373 stars 124 forks source link

Remove all of the extra libraries... #242

Open kevmoo opened 6 years ago

kevmoo commented 6 years ago

See https://pub.dartlang.org/documentation/angular_components/0.8.0/

Now that we're smart about tree shaking, we should expose only a few things outside of src?

nshahan commented 6 years ago

This is interesting but I'm not sure how I'd like to handle it right now.

Most of the components have a mixin SCSS file that is intended to be imported in client applications to customize the look and feel of the component. I'm relying on the dart implementation of SASS that allows dart package: imports in your SASS files.

I wouldn't want to move those into lib/src but I'd also like to keep the next to the component that they are for. I'm also not sure if we would want to combine them into one monolithic mixin for import.

nshahan commented 6 years ago

What is the motivating factor for hiding files in lib/src if we are exporting all of them anyway?

kwalrath commented 6 years ago

On a separate thread, @chalin noted that webdev limits the docs for the generated libraries using the dartdoc --include/--exclude flags. He thought pub docs could do the same. @kevmoo thought this might be something to add to the new dartdoc_options.yaml (see dart-lang/dartdoc#1638).

/cc @jcollins-g

jakemac53 commented 6 years ago

If anything, imo, we should move in the opposite direction of this and delete or deprecate the top level angular_components.dart import. Or, split this package into a number of smaller packages (but that would be a humongous effort).

Using a single material widget should not require you to import every single material widget we provide.

Now that we're smart about tree shaking, we should expose only a few things outside of src?

We aren't smart about tree shaking in modular compilers (dartdevc), and even in dart2js relying on that isn't a good idea. Will it remove some of the unused code? Ya, most likely. Will it remove all of it? Extremely unlikely. For instance you are probably going to get at a minimum the toString method from all transitively reachable classes (from what I understand at least).

This also affects more than just deployed JS size. It affects build times, and build dependencies which affects memory consumption, time to create analysis contexts, and asset graph size.

The package:angular_components/angular_components.dart has a convenience benefit to be sure, but the tradeoffs long term of using that are just not worth it. There is a reason this package does not exist internally and we force users to import them as separate packages.

kwalrath commented 6 years ago

There's a difference between documenting all the libraries and encouraging people to import only the libraries that they absolutely need. The API doc for each component could encourage using the small-library import, even if the API doc is only exposed in the big library.

I'm afraid that showing all the libraries in the sidenav is intimidating and will stop people from finding the ones they need. Search will help, to some extent, but it's nice to be able to have a single library overview that includes everything.

TedSander commented 6 years ago

In terms of documentation I would rather have the Gallery fill that need as it has and will have much better documentation about how to interact with an Angular component then the DartDoc will have. Not to say that hopefully the dart doc doesn't look nice, but I wouldn't want to structure code around just making the dart doc look nice.

I agree with Jake that I would rather not have the uber import. I lost that argument when we first went down this path, but maybe it is time to readdress.

chalin commented 6 years ago

We aren't smart about tree shaking in modular compilers (dartdevc), and even in dart2js relying on that isn't a good idea. ...

On a slightly related note, does that mean that we should be, in general, encouraging folks to write import ... show ...? (I vaguely recall there being a guideline that was the opposite of this).

jakemac53 commented 6 years ago

The show does not help ddc compilation, or build times, unfortunately.

Every step in the modular process would have to do tree shaking to figure out which of its dependencies could be removed because of the show/hide which would likely be prohibitively expensive.

Also in situations like bazel the dependencies have to be declared ahead of time statically so they could never be trimmed that way unfortunately.