ember-models-table icon indicating copy to clipboard operation
ember-models-table copied to clipboard

Bundle size in v4

Open kcarmonamurphy opened this issue 3 years ago • 8 comments

Hi Oleg,

I wanted to first congratulate and thank you on your incredible work with this addon. We started using it last year and it's been a pleasure to customize it to suit our needs and also nice to see that it is being actively developed.

We conducted an audit of our bundle size and noticed that this addon takes up quite a chunk of the total build size:

Screen Shot 2021-11-10 at 11 47 28 AM

Is there anything we can do to reduce this? Will v4 ship with a reduced bundle size?

Environment information:=

  • ember-models-table version: 3.4.0
  • ember version: 3.24.0
  • ember-data version: 3.24.0
  • ember-cli version: 3.24.0
  • node version: 14.16.1

kcarmonamurphy avatar Nov 10 '21 17:11 kcarmonamurphy

Hi, @kcarmonamurphy. I'm glad that you and your team found emt useful!

About your question. Well, addon's "core" itself is pretty big. Here is a default theme with its components. And models-table itself is huge too.

Removing not used themes may reduce bundle size but not to much (and it may require an own fork for you). I'm thinking about moving themes to the separated addons. It should (I hope) happens in the next v4-x releases.

Moving to v4 is going to be a big step that may require a lot of changes depends on your emt usage. Main point here is one-way binding in the v4 components. BTW, if you will try a v4 I'll be glad to get your feedback :) Since it's in beta-state it's possible to do some breaking changes.

onechiporenko avatar Nov 10 '21 18:11 onechiporenko

UPD I've checked 4.0.0-beta.1 with a clean Ember App and got next:

image

onechiporenko avatar Nov 10 '21 18:11 onechiporenko

Thanks for getting back to me so quickly, @onechiporenko. I appreciate your detailed response and commitment to the project!

Seems like v4 is indeed a nice improvement and I'd be happy to give it a test drive. Is there an upgrade guide or Changelog I could take a look at?

Yes I do think stripping out the themes as addons would be a good idea. We heavily customize emt to suit our style guide and therefore only need the basic theme.

I don't know too much about tree-shaking but hopefully there is a future where even the default theme could be customized to only provide the components that are absolutely needed.

I know that with Glimmer components only one way binding is available so that shouldn't be a problem for us. Our wrapper component is glimmer itself and we've consciously tried to avoid two-way binding where possible (except for modifying properties on the EMT column object itself (to set a filter string or sort string programmatically, as an example)

kcarmonamurphy avatar Nov 17 '21 20:11 kcarmonamurphy

@kcarmonamurphy, well... changelog doesn't exist for now. It's possible to generate typescript docs for projects locally. Later I'll add them to github-pages.

onechiporenko avatar Nov 17 '21 23:11 onechiporenko

We also really enjoy this addon, been using it for over 3 years now, I could help with some of the refactor if needed.

In theory tree shaking should work out of the box with embroider if we assume people are hinting embroider with static analysis... instead of themes with strings, you do:

import MyCustomComponent from './my-custom-component';

const DefaultTheme = {
   components: {
       'some-component': MyCustomComponent    
   }
};

Or

{{! this works because embroider can understand `component` helper with static strings }}
<ModelsTable @theme={{hash components=(hash some-component=(component 'my-custom-component))}} />

Or

<ModelsTable @theme={{hash components=(hash some-component=(this.mySomeComponent))}} />

with

import SomeComponent from './some-component';

class Component extends GlimmerComponent {
  get mySomeComponent() {
     return SomeComponent;
  }
}

And inside ember-models-table source we should be using from @embroider/util (ensure-safe-component) utility

{{#if @someComponent}}
  {{#let (component (ensure-safe-component @someComponent) someProp=10) as |SomeComponent|}}
     <SomeComponent />
  {{/let}}
{{/if}}

With this approach, I don't think you need to split in separate addons the themes, but it makes sense, maybe migrating to a monorepo approach could be great.

betocantu93 avatar Nov 30 '21 14:11 betocantu93

HI, @betocantu93. Thanks for response! I think moving from string-pathes to imports going to be a good idea.

onechiporenko avatar Dec 02 '21 16:12 onechiporenko

Digging deeper, monorepo is good option too, however I didn't work with monorepo before. Going to have a lot new experience.

onechiporenko avatar Dec 02 '21 19:12 onechiporenko

Yup, there are lots of examples out there, last I made was: ember-eui which I heavily based on the frontile repo, I still think it would be great to move away from strings in combination of using the @embroider/util package to bring static analysis (tree shaking by default)

betocantu93 avatar Dec 02 '21 20:12 betocantu93