Skip to content
This repository was archived by the owner on May 20, 2023. It is now read-only.

Remove all of the extra libraries... #242

Open
kevmoo opened this issue Mar 27, 2018 · 8 comments
Open

Remove all of the extra libraries... #242

kevmoo opened this issue Mar 27, 2018 · 8 comments
Assignees

Comments

@kevmoo
Copy link
Member

kevmoo commented Mar 27, 2018

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
Copy link
Contributor

nshahan commented Mar 27, 2018

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
Copy link
Contributor

nshahan commented Mar 28, 2018

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

@kwalrath
Copy link
Contributor

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
Copy link
Contributor

jakemac53 commented Apr 12, 2018

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
Copy link
Contributor

kwalrath commented Apr 12, 2018

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
Copy link
Contributor

TedSander commented Apr 12, 2018

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
Copy link
Contributor

chalin commented Apr 12, 2018

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
Copy link
Contributor

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants