Remove unused / barely used dependencies?

[acorncom]

I'm planning on using this add-on in a client app (thanks very much for all the work, it's great! ❤️ ) While looking things over, I was struck by the number of dependencies that were going to get included in my app:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/package.json#L25-L36

The ones that specifically caught my eye were the various helpers. Curious, I dug in to see where all those helpers were being used.

ember-composable-helpers

No longer used in add-on templates from what I can see, just used in the dummy app.

ember-cli-string-helpers

The html-safe helper is used in three places:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/lt-head.hbs#L13-L15

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/lt-foot.hbs#L6-L8

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/lt-body.hbs#L21-L23

ember-truth-helpers

The or helper is used three times in one place:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/lt-body.hbs#L2-L4

The and helper is used once:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/columns/base.hbs#L5

Should we remove these dependencies that bleed into consuming apps?

I've steered clear of using these types of helpers in client apps, as I prefer cleaner templates. I realize that's not something everyone agrees with, but until tree-shaking hits, I'd prefer to keep my asset sizes down as much as I can.

Do we want to remove these dependencies to slightly reduce asset sizes for consuming apps? Doing so could be a breaking-change if other developers have begun to rely on the helpers without specifically adding them to their own app dependencies ...

If we do want to remove the dependencies, it looks like we might want to replicate the html-safe helper (as it's being used in some loops that might be easiest to leave alone). The other calls could easily move into computed properties though ...

Interested in your thoughts! 😀

[buschtoens]

Thank you so much for digging into this and keeping a watchful eye on asset size! I'm totally on your side here. 👍

I think we can safely do the following:

ember-composable-helpers

Move to devDependencies.

ember-cli-string-helpers

Remove from dependencies and recreate the {{html-safe}} helper.

ember-truth-helpers

Move to devDependencies.

{{or}}

Replace the (or someComponent (component 'defaultComponent')) construct by setting the default value in the JS class:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/components/lt-body.js#L266-L306

{{and}}

Luckily both branches of the if statement depend on sortIconProperty, which allows us to restructure the logic:

https://github.com/offirgolan/ember-light-table/blob/0d99b0f4655d6a02d0ee1d3c1801db822922269d/addon/templates/components/columns/base.hbs#L5-L9

{{#if sortIconProperty}}
  {{#if sortIcons.iconComponent}}
    {{component sortIcons.iconComponent sortIcons=sortIcons sortIconProperty=sortIconProperty}}
  {{else}}
    <i class="lt-sort-icon {{get sortIcons sortIconProperty}}"></i>
  {{/if}}
{{/if}}

[buschtoens]

Doing so could be a breaking-change if other developers have begun to rely on the helpers without specifically adding them to their own app dependencies ...

Valid point, but since we're not 'explicitly' exporting these helpers, I think we can risk this. Googling the error message should give you pretty clear instructions on what to do.

Error: Compile Error: or is not a helper

Or do you think this is too dangerous?

[alexander-alvarez]

I think as long as we document it in the release notes it would be ok

Imagine a:

With this release -- we've grately reduced the output size of ember-light-table. We realized thanks to @acorncom that we had some large dependencies that were either in dependencies instead of devDependencies (they were only used in our demo app) or could easily be replaced with our own implementation of small helpers. Note if you've depended on any of the helpers defined in ember-composable-helpers, ember-cli-string-helpers, or ember-truth-helpers but have not added them as dependencies to your host app this change is a breaking change because your host app had a "hidden dependency" that was provided by this addon

Side note: it looks like we can tree shake at build time ember-cli-string to only bring in 'html-safe'.
https://github.com/romulomachado/ember-cli-string-helpers#configuration

[acorncom]

Side note: it looks like we can tree shake at build time ember-cli-string to only bring in 'html-safe'. https://github.com/romulomachado/ember-cli-string-helpers#configuration

I'm all for not duplicating work on that html-safe helper if we can avoid it. I won't have time until late next week at the earliest to implement these changes (have a crunch on a deadline at the moment), but yes, that message or something akin looks reasonable to me ...

Technically though, if we're sem-ver lawyering, we'd need to do a major version bump to indicate potential snags ... 🤔

[alexander-alvarez]

Technically though, if we're sem-ver lawyering, we'd need to do a major version bump to indicate potential snags ... 🤔

Fair enough.. @buschtoens are there other breaking changes we'd want to coordinate with this?

[buschtoens]

Hmmm... Probably. 😅

The first thing that comes to mind that is breaking and wouldn't require a lot of work is removing the light-table/cells and light-table/columns path prefixes, so you put your components where you like.

[ynnoj]

Any activity on this as of yet? I'm keen to start contributing 🚀

[buschtoens]

The only thing holding us back is the fact, that we might break apps, by removing these dependencies.

#524 (comment):

Technically though, if we're sem-ver lawyering, we'd need to do a major version bump to indicate potential snags ... 🤔

I would like to gather some more breaking changes, but your're PR would be a great start! 💪