-
Notifications
You must be signed in to change notification settings - Fork 12k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Angular cli - not removing dependencies of unused library components #13082
Comments
Assuming that the You can make the change locally in |
@kzc even going into node_modules/moment and changing the package.json to include that property doesn't work. I have already tired that before I created this issue :-). |
Heya, I was looking at this today. I didn't use This is how it looks like for the repro as is: The important bit is moment.js with around 333kb. The reason why moment is in the compilation is because:
Running One interesting bit is
That means that At this point it's reasonable to think At another point in the verbose log we can find this:
So for some reason, the At this point it is too late to remove That list shows where the imports are used. I added some debug code to see the referenced ngfactories:
The important bits seem to be:
At this point I am a bit confused. The In the verbose log we also see:
But no such statements for Those two are both exported and used in:
But at this point I don't think that matters since So now we can look at how webpack deals with the side-effects: https://github.com/webpack/webpack/blob/f6d67b6/examples/side-effects/README.md
This part only talks about re-exports. I think what's happening is that |
I tested those two hypothesis and as far as I can tell:
I opened an issue for the difference in the last two behaviours in webpack webpack/webpack#8597. |
Looked a bit further into that webpack issue with @clydin and he pointed out that This is analogous to what is happening today with ngfactories. As long as an import from a ngfactory is used, all components it uses will remain. Will discuss with the team what can be done about it. |
@filipesilva, dude you’re awesome!!! Your explanation really helps my understanding. This is something that I would love to find a solution for even if we have to write a specialized post processor to remove the code. We are going to start investigating as well to see what can be done in the meantime. I’ll add what we’ve been able to discover here. Thanks so much for taking a look at this. |
Tree shaking in Webpack is generally not as sophisticated as Rollup (*). For best Webpack results one should export one item per source file (default or named) and have the main library file re-export everything.
(*) With one notable exception - Rollup doesn't support the package |
@kzc, lodash only reexports or uses code that is first party which is why it properly gets removed, however, if they were to import moment or any other third party library in one of their functions that third party code will get included even if you don’t use the function that uses it. I am not sure what would happen if the third party was also in esm format. “Own code” gets properly handled if it is exported that way. I believe I have tried it with both fesm and esm formats and either way it includes transitive dependencies that aren’t used. |
@filipesilva, I just added a label for the triage meeting this week. |
My previous comment was not directed towards moment in particular, but would be a workaround for the webpack deficiency seen in https://github.com/filipesilva/webpack-side-effects. |
Heya, just wanted to mention that I brought this up with the team. It's working as intended insofar as that's the way that NgFactories have always behaved, and why Angular Material ships many individual modules. This might change with Ivy (the new compiler engine) though. I tried using it now and moment wasn't included. By v8 we should have an opt-in version of Ivy for everyone to try. |
Not only that, I have a similar problem where all of my transitive dependencies are included even when importing a single class, re-exported in public_api.ts of an Angular library project into my main project. Edit: src/lib/component/model.ts src/public_api.ts consuming InputModel: Library's package.json contains
|
@p0ma unless your library is free from side effects, and your transitive dependencies are also free from side effects, it is unlikely that they will all be tree shaken. This is how ES modules work. You must exercise caution when adding dependencies to a library because you are passing them on to your consumers. |
@filipesilva Thanks for your reply. I got confused even more though. Because not only I get my transitive dependencies included, but also all the unused shared components go into the bundle as well. Should I remove that class import, I get whole another picture. |
Hello @filipesilva @p0ma, My case is the following:
In the consumer angular project I import only 1 module like: But prod build with the following config:
does include all code from angular library. Is there a solution for this?
Please advice what can be wrong. |
@p0ma @yansokalau if you provide a repro I can look into it, but without one it's very hard to say anything concrete. |
Hi @filipesilva Thanks in advance, the problem does really mean a lot, so me and my team looking forward for resolving it. |
hi @babeal, do you have solution for the issue? |
Hello @filipesilva, |
Hello @yansokalau I have the exact same problem as you. Even though I'm using only one module from a library, my application imports all the components, modules and external dependencies. Did you find a solution for your project ? Thanks in advance |
Hi @schallus, |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Bug Report or Feature Request (mark with an
x
)Command (mark with an
x
)Versions
Npm: 6.4.1
Angular CLI: 7.1.0
Node: 10.14.0
OS: darwin x64
Angular: 7.1.1
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Package Version
@angular-devkit/architect 0.11.0
@angular-devkit/build-angular 0.11.0
@angular-devkit/build-ng-packagr 0.11.0
@angular-devkit/build-optimizer 0.11.0
@angular-devkit/build-webpack 0.11.0
@angular-devkit/core 7.1.0
@angular-devkit/schematics 7.1.0
@angular/cli 7.1.0
@ngtools/json-schema 1.1.0
@ngtools/webpack 7.1.0
@schematics/angular 7.1.0
@schematics/update 0.11.0
ng-packagr 4.4.0
rxjs 6.3.3
typescript 3.1.6
webpack 4.23.1
Repro steps
The angular build/packaging process is unable to remove third party dependencies from unused components in a library. The example provided includes a library called components containing a component that imports moment.js and one that doesn't. When only the component that DOES NOT use moment is imported into the application the resulting application bundle will still contain moment. I have tried to force the build process to use esm5 instead of fesm 5 and this does not help. The only thing left referencing moment would be the metadata.json. This problem makes it difficult to build large component libraries using the cli due to the fact that all runtime transitive dependencies will be included in the final bundle whether they are used or not. I would like to know why this is and where the problem resides.
https://github.com/babeal/ng-tdp
clone, run npm install, then npm run analyze
The log given by the failure
The results of the webpack analysis will show moment in the main bundle as well as inspecting the minified main.js in the dist folder.
Desired functionality
Should not include unused external dependencies into the final bundle
Mention any other details that might be useful
The text was updated successfully, but these errors were encountered: