-
Notifications
You must be signed in to change notification settings - Fork 12k
feat(@ngtools/webpack): show warning when a TS compilations contains … #15061
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
Conversation
Note: tests to be added Also without implementing this #15030 for VE the warning will be shown all the time for newly generated apps. This is because by default we do include redundant files in the compilations. There are;
NB: All existing projects will have this warning. |
We may want to ignore anything involved in file replacements. |
Only the last one, is in fileReplacement but, why should we ignore it? It is in the compilation because of the non strict includes that we currently have as otherwise they won’t be part of the compilation. |
As discussed we should only enable this for Ivy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how this ended up not requiring a lot of changes.
Most of my change requests are actually questions.
return [...uniqueDependencies] | ||
.filter(x => !!x) as string[]; | ||
// Add used source files | ||
this._usedSourceFiles.add(forwardSlashPath(fileName)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhat reluctant on adding to the list here. I agree that it's a convenient place to hook into, but it adds somewhat unrelated things to the mental model for this call.
Would this approach work as well if added to getCompiledFile
instead (assuming adding imports is not needed either)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checked and getCompiledFile
will not be called for all files, such as type only files.
So if we want to remove this from here, what we can do is to expose the _usedSourceFiles
publicly and from the loader in https://github.com/angular/angular-cli/blob/master/packages/ngtools/webpack/src/loader.ts#L77 we do:
Ex:
[...new Set(dependencies)].forEach(dep => {
plugin.updateChangedFileExtensions(path.extname(dep));
this.addDependency(dep);
plugin.usedSourceFiles.add(forwardSlashPath(fileName));
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding an additional Webpack compilation hook inside the Angular plugin that compares the set of TS source files with webpack modules? Not sure of the appropriate hook off the top of my head though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s sound like a good idea I think the hook was called finishedModules
or something like that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@clydin I tried the above, but the problem is that type only files will not be part of the webpack modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Managed to get the information needed through buildInfo.
…unused files When a tsconfig without `includes` or `files` all files under the `rootDir` will be included in the compilation. This results in redundant files to inserted as part of the ts compilation which in some cases might reduce the compilation drastically. Related to: TOOL-949
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. |
…unused files
When a tsconfig without
includes
orfiles
all files under therootDir
will be included in the compilation. This results in redundant files to inserted as part of the ts compilation which in some cases might reduce the compilation drastically.Related to: TOOL-949