Clarification on Ionic /components folder

#1

The google doc for lazy loading suggests:

One module that imports all component classes, no individual modules for each component
`

  • components/
    - components.module.ts (ComponentsModule)
    - imports and exports all components the user creates
    `

Can anyone provide clarification on this? What should components.module.ts look like? Do I include the components module in the declarations: [] array in app.module.ts? Are these components then also lazy loaded?

Side question: I presumed lazy loaded in all these cases meant that modules were imported completely when first requested via an http request. This doesn’t appear to be the case (webpack is still bundling everything). Is lazy loading in this sense referring to the entire codebase being packaged but angular does not process the modules into existence (internal representation) until requested?

Thanks!

1 Like
Ion-icon produce ion-null-icon- (Resolved)
Ionic angular 4 project understanding
NavPush string not working
#3

Not last I checked.

I guess that’s relatively accurate, but to be pedantic, angular doesn’t know anything about it. Ionic cooperates with webpack’s code splitting.

#4

I’m having trouble with it. Here’s my components.module.ts:

import { NgModule } from '@angular/core';
import { MakeModelYear } from './make-model-year/make-model-year';
import { ClaimsBaseCollection } from './claims-base-collection/claims-base-collection';

@NgModule({
  declarations: [
      MakeModelYear,
      ClaimsBaseCollection
  ],
  exports: [
      MakeModelYear,
      ClaimsBaseCollection
  ]
})
export class ComponentsModule { }

But now I’m getting “Can’t bind to ‘ngModel’ since it isn’t a known property of ‘ion-input’.”

I’ve included FormsModule from @angular/forms in the main app.module.ts, along with importing the components.module.ts. Any idea what could be causing that? It’s like it doesn’t recognize ngModel as an angular directive.

#5

Yes, but I don’t have the authority to say anything official about it.

If you want my unofficial opinion, if you have custom components you are better off simply declaring them in your AppModule. I am perfectly willing to be corrected on this point, but I find the official recommendation of making a module for all custom components and importing it in all pages that use any of them to be pretty much the worst of all possible worlds. It exacerbates code duplication and facilitates problems such as you are seeing here. If you insist on persisting with pursuing it, I would suggest importing IonicModule in your ComponentsModule.

1 Like
#6

Yea I had to import both FormsModule and IonicModule in that components.module.ts. I imported components.module.ts in app.module.ts. I don’t think I needed to import components.module.ts in every page that uses the component, because it works on pages that have the custom components by nature of it being included in app.module.ts (the root app). It does seem weird to me to import it into every page individually. The only reason I could see for that is because the modules themselves are being lazy loaded, that implicitly the components.module.ts will also not be loaded until the first page that needs it… Seems likely?

#7

At that point I don’t see the point of having a ComponentsModule in the first place over just putting all the custom components in the AppModule.

That was what I was hoping for and would be much better (at least for me) than the current state of affairs. What happens now is that a separate complete version of the code in ComponentsModule ends up in every page chunk that imports it. It bloated my app binary by about a third and delivered no measurable benefit to startup time, so I decided to ditch lazy page loading for now. YMMV.

1 Like
#8

So further issues:

Like you said, I moved the custom components back into my app.module.ts. But it’s not recognizing them. It’s saying model-make-year is not recognized. I’m importing it and then have it in the imports: [] array, but it’s still not getting it. It’s exactly the same as I had it in my ionic2 app but now it doesn’t seem to like it.

In app.module.ts:

import { MakeModelYear } from '../components/make-model-year/make-model-year';
import { ClaimsBaseCollection } from '../components/claims-base-collection/claims-base-collection';


@NgModule({
    declarations: [
        MyApp,
        MakeModelYear,
        ClaimsBaseCollection

And then in MakeModelYear.ts:

@Component({
    selector: 'make-model-year',
    templateUrl: 'make-model-year.html'
})
export class MakeModelYear {

But for some reason it’s throwing the error that it does not know what make-model-year is. Do I need to put this in the declarations: [] of the page that includes it? Is this a new property of lazy loading the pages (and having them be their own modules?) I’ll have to include MakeModelYear in the page’s .module.ts instead of in the top level app.module.ts? That makes it seem like the whole including components.module.ts in every module.ts is the ONLY way to do it if I want to lazy load pages. Is webpack really that dumb as to include a copy of components.module.ts for each import in separate files?

#9

@danbucholtz if you don’t mind, this is a follow-up on the question you answered for me a few days ago. Could you (or someone else on the team) please address this?

Is there something in the pipeline to address this? Or is there a reason this approach is a good thing despite surface appearance? Or is this just the way things are going to be for a long time, and if we want to lazyload we have to deal with it? I’m running out of pre-beta things to do, so I need to decide soon whether I am going to lazyload or not, and I’d like to know what the plan is on your end, going forward. Thanks very much.

#10

Ruh roh, now the hall monitors are going to kick my ass.

#11

No, the imports array is only for other modules. In addition to declaring it in AppModule, is it also listed as an entryComponent? If you don’t do that, ngc won’t recognize it’s needed and will drop it on the floor.

#12

Yea, sorry I wasn’t clear on that. I was putting my components.module.ts in the imports array. I ended up going with their recommendation (as this is just a prototype) despite your warnings, hoping that MyMMV with webpack bloating my compiled bundle. Since all my pages are now lazy loaded with all their own .module.ts files, adding components.module.ts to the imports of app.module.ts didn’t do anything and it was necessary to add it to the imports of any page.module.ts that requires it. Thanks for the help though. My upgrade to ionic 3 is complete and allowed me to solve my other problem that you had answered while helping me understand what’s actually happening with the imports and giving every page it’s own module.

#13

I checked my output compiled files and I do see that it is duplicating the import in each page that imports components.module.ts, so you are correct. Like I mentioned, this is fine for my prototype (and technically since the pages including these components are lazy loaded, by proxy the components are lazy loaded… but that doesn’t take in the negative of code duplication).

Is there any way to prevent this though? It seems strange that the code splitting is happening, but it’s not smart enough to not inline the imports… Why wouldn’t it just use the separately compiled components.module.ts file and http that in first and THEN load the lazy loaded page that requires it, passing the reference in (like require.js, it is cached after load, or loaded via http if it’s the first time the module has been requested). Each page imports ionic-angular and various other things but those do NOT get inlined, unlike my components module…

#14

Apparently it’s a tough enough problem and/or falls between the cracks between webpack and the build systems that use it that nobody’s managed to solve it yet. Strictly speaking, it’s not just Ionic. Angular-cli apps have the same issue.

There isn’t any "http"ing happening. If you’re interested in the internals, start by looking at deep-linker.ts.

I believe those are in the main app module, which is where I was trying to get you to put all your custom components as well. It doesn’t sound like that was successful.

#15

I get that in app.module.ts there are things being imported and not inlined. But what I’m confused about is that my lazy loaded pages have a page.module.ts now (due to lazy loading) and they import things like NgModule, and IonicPageModule, and then my custom ComponentsModule (components.module.ts), and the only import that gets inlined by webpack is the components.module.ts. So how are IonicPageModule and NgModule, for instance, not being inlined but my custom ComponentsModule is?

#16

I see now that the CommonChunksPlugin that webpack uses to decide what not to inline only respects things in node_modules, and a specific subset of such:

export const NODE_MODULES = join(process.cwd(), 'node_modules');
export const RXJS = join(NODE_MODULES, 'rxjs');
export const ZONEJS = join(NODE_MODULES, 'zone.js');
export const ANGULAR = join(NODE_MODULES, '@angular');
export const IONIC = join(NODE_MODULES, 'ionic-angular');

Unfortunately as ionic consumers we don’t get access to this webpack config which is buried in @ionic/app-scripts. If I could add my components.module.ts to this it would consider it a common chunk and stop inlining it where it is imported in each page’s .module.ts file.

1 Like
#17

That’s a great find. I was coming at it from a different direction, and discovered that this problem doesn’t just affect components. Things like ionic-storage (and presumably all native plugin shims, although those are pretty small) also get duplicated in all lazily loaded pages that reference them.

#18

Actually, you do. You can copy it to (for example) ./my-webpack.config.js, hack it up and then reference it using the ionic_webpack config setting documented in here.

EDIT: never mind, I now see that you’re not talking about the ordinary build webpack config, but rather a plugin, so that’s going to be harder to override.

EDIT 2: and @rlouie might be interested in this: based on your earlier sleuthing, I did take a look at this, and at first blush it seems like one can have more than one of these critters in a project. If so, it could theoretically be possible to decouple the code splitting structure from the deeplinker, allowing lazy loading of chunk-by-feature and ameliorating the code duplication problem somewhat.

1 Like
#19

Good info on customizing the webpack config by overriding it in the package.json. I wonder if you could create a custom webpack that overrides the one plugin the default one has (ionicWebpackFactory.getIonicEnvironmentPlugin()) and replace it with something local to the project that instead uses a custom chunking plugin. For instance, this is the ionicWebPack factory:

So you could potentially have a custom version of that which supplies another function getCustomCommonChunksPlugin or something that checks some “chunks.config” in your project and makes sure to add those specified files as common chunks to code split?

#20

Ya, I spent a good while messing with commons chunk plugin in vanilla Angular to see if I could get rid of your duplicate code issue. It was a good learning experience but ultimately unsuccessful. The problem (I believe) was that the Angular compiler took care of the code splitting in it’s own way rather than using normal chunks so it didn’t recognize the lazy loaded bundled code.

So in vanilla Angular I ended up with main.js, vendor.js, 01.js and 02.js. Regardless of any settings it just didn’t recognize the 01.js and 02.js. I believe in order to make it work with that comons chunk plugin you’d need an entry point for each lazily loaded module.

It’s possible I missed something but I went pretty deep into it. Oh, and yes you can have multiple of them in a project, the vanilla Angular CLI config uses multiple out of the box, but in this case it doesn’t seem to help unfortunately. Maybe it works differently in Ionic, but I seriously doubt it as I’m fairly confident it uses the same Angular compiler behind the scenes.

EDIT: I’m sure that’s why in the github issue on Angular their answer is specifically that they need to use a dependency tree when bundling to see several layers deep. If it was as simple as adding another commons chunk config into their webpack config I’m sure they just would have done that.

1 Like
#21

Here’s my .02 on this for what it’s worth (and the Angular team’s honestly). You should group your code by feature not by type. If your component is shared across the most of the app put it in a shared feature module: https://angular.io/docs/ts/latest/guide/style-guide.html#!#04-10. It will be eagerly loaded and it will be very clear what components are shared by your entire app.

Then you continue to group your code by feature: https://angular.io/docs/ts/latest/guide/style-guide.html#!#04-07. So, for example let’s say user profile is one of your features. Make a new folder named profile, create a profile module in it. Let’s say you have a profile photo component that lets users upload, crop, and set a profile photo. That’s only going to be needed by this feature, so in that same folder create profile-photo.component.ts and import it into the profile module. You’d also presumably create a profile service and import it into that module as well.

Now you have a nice self contained reusable module that’s isolated from the rest of your code and can easily be tested and reused. You’ve also eliminated duplication of component code because your profile photo component is only loaded with the profile feature, rather than being copied into every single lazy loaded module.

Now, there’s still an issue here a little because as I’ve complained about already Ionic only lazy loads individual pages, so if your profile feature needed two pages you’d be in trouble. All this really comes from the Ionic team going against Angular best practices and encouraging archaic design patterns like grouping by type instead of feature.

If Ionic supported lazy loaded modules instead of pages (they are working on this) this issue would be very minimal because most components you’d want to lazy load would be specific to a feature area of the app, but I’ve ranted enough on that topic already :slight_smile:

3 Likes